Closed compulim closed 9 months ago
alt
be a non-boolean attribute? Does it matter for this code change?=""
?
=""
, create a new option (potentially with default values) that lets the end user define what tags can have this value - like the allowedAttributes
option.Finally, please make use of the Discord to ask questions. Try to answer the questions using internet resources, but don't be afraid to ask questions on the Discord about anything. We are here to help!
Hi. I am new here and just had a look at this issue. The 2nd case above seems correct to me - alt
is implicitly alt=""
. Is that right? (unless we actually want to specify alt=""
)
// EXPECT: <img alt="" src="https://example.com/" />
// ACUTAL: <img alt src="https://example.com/" />
console.log(
sanitizeHtml('<img alt="" src="https://example.com/" />', {
allowedAttributes: { img: ['alt', 'src'] },
allowedTags: ['img'],
nonBooleanAttributes: []
})
);
@zhna123 - There are cases where the image being added is purely decorative. In that case, we don't want explicit alt
text. However, for your HTML to be in spec and pass validation, it needs an alt
attribute. Leaving that attribute as a boolean without string will cause some screen readers to output "no alt text" and others will output the filename of the image. Both of these are disrupting to the UX of the page. So, the OP is correct, the alt
tag should be allowed to be set to an empty string.
I agree it would make sense to fix this one specifically for the alt attribute.
On Tue, Sep 26, 2023 at 6:27 AM Robert Means @.***> wrote:
@zhna123 https://github.com/zhna123 - There are cases where the image being added is purely decorative. In that case, we don't want explicit alt text. However, for your HTML to be in spec and pass validation, it needs an alt attribute. Leaving that attribute as a boolean without string will cause some screen readers to output "no alt text" and others will output the filename of the image. Both of these are disrupting to the UX of the page. So, the OP is correct, the alt tag should be allowed to be set to an empty string.
— Reply to this email directly, view it on GitHub https://github.com/apostrophecms/sanitize-html/issues/633#issuecomment-1735261269, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27OAQN37VYC6CLHLMETX4KUXZANCNFSM6AAAAAA4RB5HGI . You are receiving this because you are subscribed to this thread.Message ID: @.***>
--
THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his
Okay. I used VoiceOver to test alt
without explicitly specify =""
, it didn't announce anything, but removing it will be "unlabeled image". But it's true that I can't guarantee all the screen reader will behave the same way.. I created a pr to fix this.
PLEASE NOTE: make sure the bug exists in the latest patch level of the project. For instance, if you are running a 2.x version of Apostrophe, you should use the latest in that major version to confirm the bug.
To Reproduce
Step by step instructions to reproduce the behavior:
Expected behavior
alt=""
should be left as-is.Describe the bug
Empty
alt
attribute means the alt text of the image is intentionally left blank. For example, border graphics may not needalt
attribute if they are unimportant decoration.When
alt
is an empty string, screen reader will skip the element and read nothing.When
alt
is not set, screen reader will read "image".So,
alt=""
is not same asalt
or unset.To workaround the issue, use
<img role="presentation" src="https://example.com/" />
.Details
Version of Node.js: 18.16.0 PLEASE NOTE: Only stable LTS versions (10.x and 12.x) are fully supported but we will do our best with newer versions.
Server Operating System: Ubuntu 20.04 on WSL The server (which might be your dev laptop) on which Apostrophe is running. Linux? MacOS X? Windows? Is Docker involved?
Additional context: Seems regression of #529.
Screenshots If applicable, add screenshots to help explain your problem.