Bug 25524

Created attachment 29966 [details] test case Here's a test case, should one be of use. Note that there is an image included with both alternative text and a title attribute. At the moment, WebKit is exposing the alternative text as the accessible description which means it conflicts with my recommendation. :-) A couple of alternatives for the alt text for your consideration: 1. Expose the alt text as the name. This is what Gecko does FWIW. 2. Expose the alt text as the image description (i.e. rather than the accessible's description). Please see http://library.gnome.org/devel/atk/unstable/AtkImage.html Personally, I could go either way. My thang has always been the consistency/reliability factor: If I know where I can expect to find/access something I'm good. :-) Will, do you have a preference/thoughts on which approach is preferable?

Sorry for the spam. If there's a way to create a new bug and *at that time* add the attachment *and* set the keyword, please let me know. Thanks!

(In reply to comment #0) > [...] > Therefore, it is recommended that the content of the title attribute be exposed > as the object's accessible description. Trying to reproduce the bug with the test case provided I found out this is now already fixed, so I guess it would be great if you could verify it to make sure this bug is not here anymore :-). (In reply to comment #1) > [...] > Note that there is an image included with both alternative text and a title > attribute. [...] A couple of alternatives for the alt text for your > consideration: > > 1. Expose the alt text as the name. This is what Gecko does FWIW. This is the alternative currently implemented, as far as I could see. Btw, commit ID where I tried this is 5e4b39b07aa0abb24b5c219cf39d605343368f54, from 2009/07/15.

(In reply to comment #3) > (In reply to comment #0) > > [...] > > Therefore, it is recommended that the content of the title attribute be exposed > > as the object's accessible description. > > Trying to reproduce the bug with the test case provided I found out this is now > already fixed, Really?? > so I guess it would be great if you could verify it to make sure > this bug is not here anymore :-). When I look in Accerciser at the image, I see the following: 1. The image has no accessible name. 2. The accessible description of this object is "Orca" ("Orca" is the alt text; not the title text.) 3. The title information ("The logo of Orca, screen reader for the GNOME desktop enviornment") is not exposed to ATs from what I'm seeing. What I expect to see is the following: 1. The image would derive its name from the alt text (i.e. the accessible's name would be "Orca") 2. The accessible description of this object would be "The logo of Orca, screen reader for the GNOME desktop enviornment" In addition: 1. The "phone number" link has a title attribute ("i.e. the number of your phone :-)"). I do not see that being exposed to ATs. I expect the title attribute to be exposed to ATs -- and done by making that information the accessible description. 2. The entry in which the user is expected to type his/her phone number has a title attribute ("Enter your phone number without any punctuation"). Again, I'm not seeing that information exposed to ATs. (And again, I'd expect it to be exposed via the accessible description.) 3. Ditto for the Submit button (title attribute which is not exposed to ATs, but should be: "If you need a tooltip to tell you to press this button, we don't want your phone number." -- Yeah, I was tired when I made the test case. ;-)) commit 8b7420eab5c08e533279382990d1bc94b9f52360 Date: Thu Jul 16 21:52:02 2009 +0000

(In reply to comment #4) You're right, I've tried it out now and seen the bug is actually not fixed yet. I guess what happened is that I have probably run 'epiphany' outside my 'jhbuild shell' so it loaded the gecko version (installed in my system) instead the webkit one. That explains why I've seen everything correct in Accerciser :-). Terrible and silly mistake, I should have double checked that before posting that's already fixed, sorry.

Created attachment 32918 [details] Patch proposal Exposed 'alt' attribute from images as accessible name and the 'title' attribute from any HTML element as accessible description.

Created attachment 32933 [details] Patch proposal, with ChangeLog Attaching the patch again, this time with ChangeLog prepared and setting the review:? flag. Sorry for the spam.

Comment on attachment 32933 [details] Patch proposal, with ChangeLog > +2009-07-17 Mario Sanchez Prada <msanchez@igalia.com> > + > + Reviewed by NOBODY (OOPS!). > + > + https://bugs.webkit.org/show_bug.cgi?id=25524 > + [Gtk] Expose the title attribute to assistive technologies > + > + Expose 'alt' attribute from images as accessible name. > + Expose the 'title' core HTML attribute as accessible description. > + > + * accessibility/AccessibilityRenderObject.cpp: > + (WebCore::AccessibilityRenderObject::stringValue): > + (WebCore::AccessibilityRenderObject::accessibilityDescription): > + The indentation here is completely off. You should align everything at 8 chars, with no tabs at all. > + if (node && node->isHTMLElement()) { > + const AtomicString& alt = static_cast<HTMLElement*>(node)->getAttribute(altAttr); > + if (!alt.isEmpty()) > + return alt; Bad indentation here (2 instead of 4 spaces). If you're using Emacs you may want to do M-x set-variable <RET> c-basic-offset <RET> 4 <RET> =) > + } else { > + Node* node = m_renderer->node(); > + if (node && node->isHTMLElement()) { > + const AtomicString& title = static_cast<HTMLElement*>(node)->getAttribute(titleAttr); > + if (!title.isEmpty()) > + return title; > + } Bad indentation here too. I could just fix this stuff myself, and commit, since the rest of the patch looks good, but I'd like to have an updated patch submitted by you that I can just apply and commit. Thanks for your work!

Created attachment 32944 [details] Patch proposal, indentation fixed Fixed indentation problems

Comment on attachment 32944 [details] Patch proposal, indentation fixed r=me

Landed as r46038.

Comment on attachment 32944 [details] Patch proposal, indentation fixed This caused regressions: http://trac.webkit.org/changeset/46038 http://build.webkit.org/results/Leopard%20Intel%20Debug%20(Tests)/r46051%20(2931)/results.html About to roll this out.

(In reply to comment #12) > (From update of attachment 32944 [details]) > This caused regressions: > http://trac.webkit.org/changeset/46038 > > http://build.webkit.org/results/Leopard%20Intel%20Debug%20(Tests)/r46051%20(2931)/results.html > > About to roll this out. Rolled-out in http://trac.webkit.org/changeset/46056 The following tests failed: accessibility/img-aria-button-alt-tag.html accessibility/input-image-alt.html

Pasting a discussion I've had with Mario so it won't get lost in the ether. Please note that 'img' can be anything. One of the test that failed for example relates to an <img> with a button role so assuming that img as just an img would be inconclusive. We need to check those two tests for the expected behaviours. -Start---------------------------- 10:00 < msanchez> janm: around? 10:02 < janm> msanchez: hey 10:05 < msanchez> hi, just seen the regressions about bug 25524 and I was wondering (not sure though) whether the point there could actually be to change those two failing test cases 10:05 < msanchez> I mean... so far 'alt' was used for accesible descriptions 10:05 < msanchez> but with that implementation, and supposing it's correct (I think that's what gecko does, btw), 'alt' -> accesible name / 'title' -> accesible description 10:06 < msanchez> and those test cases are failing precisely because they expect the 'alt' attribute to be exposed as 'name'... 10:06 < msanchez> what do you think? 10:09 < janm> msanchez: what does the spec say atm? If the spec is not clear, we probably would want to follow FF 10:10 < janm> msanchez: If we need to change the expected results for those two tests, I think we need to engage webkit-dev as well so other ports are aware of the change 10:10 < janm> msanchez: though you might need to do more and update the mac port as well 10:11 < janm> msanchez: (if necessary) 10:11 < msanchez> hmm, don't know, I've just followed indications at https://bugs.webkit.org/show_bug.cgi?id=25524#c1 10:14 < janm> yeah and that's fine. :) 10:14 < janm> msanchez: "In order to be perceivable, elements with a role of img SHOULD have alternative text or a label associated by the accessible name calculation." 10:19 < janm> msanchez: not too sure about if title = accessible description should only apply to imgTags 10:20 < msanchez> janm: actually with that patch it was applying to every element, as it's a core attribute which theoretically should be used for giving extra detail (descriptions) over elements 10:21 < msanchez> http://www.w3schools.com/tags/att_standard_title.asp 10:27 < janm> msanchez: WAI-ARIA seems to suggest that 'title' can be used as the accessible name too http://www.w3.org/TR/wai-aria/ 10:31 < msanchez> hmm.. true, perhaps it should be used as a fallback when no other atribute, more suitable for the accesible name, is present 10:32 < janm> msanchez: yeah 10:32 < msanchez> not sure though... I'll keep thinking on that, now I have to go (2.30 am here). It was nice chatting to you, if you come up with a definitive solution to this, it would nice you wrote it down in the bugzilla :-) 10:34 < msanchez> see you -End----------------------------

(In reply to comment #14) > 10:09 < janm> msanchez: what does the spec say atm? If the spec is not clear, > we probably would want to follow FF There are occasions in which FF does some things which make it harder -- not better -- for assistive technologies. IMHO, the way in which they handle images which have both alternative text and a title attribute would be one of those times. If the spec is not clear, perhaps it would make sense for all interested parties to participate in the writing and/or clarification of the spec needed for implementation.

(In reply to comment #15) > There are occasions in which FF does some things which make it harder -- not > better -- for assistive technologies. IMHO, the way in which they handle > images which have both alternative text and a title attribute would be one > of those times. > > If the spec is not clear, perhaps it would make sense for all interested > parties to participate in the writing and/or clarification of the spec needed > for implementation. After reading the section "4.2.7 Accesible Name Calculation" at http://www.w3.org/TR/wai-aria/#nameref, it's my understanding that title should be used only as the last resort when trying to set up the accessible name for a given content: A. Elements may specify their text equivalent in DOM attributes, used in this order of preference: * [...] * [...] * Any non-tooltip native markup (such as an img element's alt attribute in HTML), or the subtree for a label pointing to this element (such as label element's for attribute in HTML) contributes according to rules defined by the appropriate specification for that markup. B. [...] C. [...] D. The last resort is to use text from tooltip markup (such as the title attribute in HTML). This is used only if nothing else, including subtree content, has provided results. However, perhaps I'm missing something here so I dare not to give an strong opinion here :-), but with this information I suppose a good option would be to go ahead with the proposal of: * Use 'alt' value for accesible name if apropriate and if it exists. If it does not exist, fallback to 'title' for the accessible name, if present. * If the accesible name is already set without using 'title', use this one for the accesible description in case there's not a better candidate for that (aria-label, aria-description). What do you think? Does this make any sense to you? In case it does, I guess the next thing to do would be, as Jan pointed out, to send a mail to webkit-dev talking about this issue.

Consider the following: <form> Phone: <input type="text" title="Enter your number without dashes." /> </form> I see stuff like this all of the time. The assumptions made by the content provider presumably are: 1. The user can see (and thus knows "Phone:" is the functional label for the field). 2. The user uses a mouse and will therefore see the the helpful title information as they hover the mouse over the field they are about to fill out. What a screen reader user wants to hear and/or see in braille is something to the effect of: "Phone: text" Depending on the user, he/she might also want to hear/see: "Enter your number without dashes." (Which is why we need this information exposed.) Unfortunately, the hypothetical content provider didn't take the time to surround 'Phone:' in <label></label>. So even if bug 25530 were fixed, this text field would have no name. What a screen reader can do (and what Orca does, and what I believe some of the Windows screen readers do) in this case is attempt to "guess" what the functional label is. For the example above, Orca would guess "Phone:" and present "Phone: text" to the user. So far so good. Now enter this standard and what I believe you are proposing: 1. The form field has no name. 2. The form field has a title. 3. Ergo, make the accessible name: "Enter your number without dashes." When a screen reader -- at least Orca -- comes across a form field that has an accessible name, it doesn't try to figure out where that name came from. Nor does it attempt to guess a name/label for a named/labeled field. It assumes the name is valid and presents it. As a result, when a user who is blind Tabs into the above field, what he/she will be presented with is: "Enter your number without dashes. text" Now the user must try to work out what number should be entered without dashes. Phone number is likely, but it might be social security number, or ID number, or order number, or subscriber number depending on the form being filled out. In order to figure out what information is expected, the user must now look around (non-visually) for the functional label. In other words, in an effort to make things more accessible, falling back on the title text makes them less accessible. :-( If, instead, the title attribute were exposed as the accessible description, a screen reader like Orca could guess the functional label and present the title information to the user upon request and/or based upon a user-configurable option. As for images: I feel less strongly about falling back on the title for the accessible name. That said.... If the alt info (if any) is always exposed as the accessible name and the title info (if any) is always exposed as the accessible description, it is quite easy for a screen reader to: 1. Present whatever information happens to be present for the image (i.e. look first to the name and failing that, the description). 2. Implement a setting so that users could opt to hear images with alt text, but skip over images that lack alt text -- even if they have title text. 3. Present detailed "where am I?" information that presents the alt text (or the absence of alt text) and the title text (or the absence of title text) to the user and by doing so helps the user better understand exactly what is on the screen. In other words, the screen reader is perfectly capable of doing the "fall back to the title as the name" thing. And if you leave it up to the screen reader to do the fallback rather than doing it yourself, the screen reader is able to provide a more consistent experience, customizable to best suit whatever the needs of the user happen to be.

(In reply to comment #17) > [...] > In other words, the screen reader is perfectly capable of doing the "fall back > to the title as the name" thing. And if you leave it up to the screen reader to > do the fallback rather than doing it yourself, the screen reader is able to > provide a more consistent experience, customizable to best suit whatever the > needs of the user happen to be. This makes a lot of sense to me as well, and your examples clarified a lot the situation and why falling back to title for the accessible name is not a good idea at all. Thanks for taking your time for so a good explanation :-) Therefore, if I understood it well, what you're proposing is going ahead with the original idea implemented in the attached patch: * Always expose 'alt' in images as the accesible name * Always expose 'title', for every HTML item, as the accesible description So, unless I had understood it wrong, this would mean that changing those test cases would be required, as they expect 'alt' to be the accesible description and not the name. Can you confirm this before I send a mail to webkit-dev proposing this change?

(In reply to comment #18) > Therefore, if I understood it well, what you're proposing is going ahead with > the original idea implemented in the attached patch: > > * Always expose 'alt' in images as the accesible name > * Always expose 'title', for every HTML item, as the accesible description That's what I'd like to see done. > So, unless I had understood it wrong, this would mean that changing those test > cases would be required, as they expect 'alt' to be the accesible description > and not the name. I'm not familiar with the WebKit regression tests or what's being done in terms of WebKitGtk+ accessibility versus platform-independent accessibility, so I'll leave it to someone else to comment. Thanks!

(In reply to comment #19) > [...] > That's what I'd like to see done. Great, so we "just" need to decide about the unit tests then :-) > [...] > I'm not familiar with the WebKit regression tests or what's being done in terms > of WebKitGtk+ accessibility versus platform-independent accessibility, so I'll > leave it to someone else to comment. Neither am I... yet :-), just getting started atm, so I agree someone else's opinion on this would be very valuable. Sending a mail to webkit-dev now then. Thanks!

(In reply to comment #19) > I'm not familiar with the WebKit regression tests or what's being done in terms > of WebKitGtk+ accessibility versus platform-independent accessibility, so I'll > leave it to someone else to comment. > > Thanks! About this particular bit... we want to be able to run all crossplatform tests (when bug #25989 is fixed), and we also have some gtk+ specific tests in place to test our own stuff (for example, I added tests to check that we do the right thing for the ATK get_text_{at,after,before}_offset methods). In this particular case it looks like we want to modify the crossplatform behavior, so we'd need to change the crossplatform tests if we convince everyone else.

(In reply to comment #21) > (In reply to comment #19) > > I'm not familiar with the WebKit regression tests or what's being done in terms > > of WebKitGtk+ accessibility versus platform-independent accessibility, so I'll > > leave it to someone else to comment. > > > > Thanks! > > About this particular bit... we want to be able to run all crossplatform tests > (when bug #25989 is fixed), and we also have some gtk+ specific tests in place > to test our own stuff (for example, I added tests to check that we do the right > thing for the ATK get_text_{at,after,before}_offset methods). In this > particular case it looks like we want to modify the crossplatform behavior, so > we'd need to change the crossplatform tests if we convince everyone else. Though I think it would be OK to ifdef this specific case, if we agree to disagree on this, and keep these themes skipped in GTK+, having GTK+-specific tests instead.

Ping. (Where are we with this?)

Chris, can we get your opinion on this? The executive summary is that Mario implemented an awesome solution (thanks!) which I would think would make things better for all platforms. It was committed, but subsequently backed out due to two tests not getting their expected results: (In reply to comment #13) > (In reply to comment #12) > > (From update of attachment 32944 [details] [details]) > > This caused regressions: > > http://trac.webkit.org/changeset/46038 > > > > http://build.webkit.org/results/Leopard%20Intel%20Debug%20(Tests)/r46051%20(2931)/results.html > > > > About to roll this out. > > Rolled-out in http://trac.webkit.org/changeset/46056 > > The following tests failed: > accessibility/img-aria-button-alt-tag.html > accessibility/input-image-alt.html Three months have passed, but there's not been any verdict regarding the tests as I understand it. If need be, I'll see if I can convert Mario's solution into a platform-unique one that doesn't impact any tests. But if you'd think these changes would be of benefit to your users as well, perhaps we can modify the tests.... Or something.... Thanks!!

From what I gather, it seems that gtk is not matching the correct data to the right interface. I think all the pieces are in place already, however and this problem can be fixed on the platform side String AccessibilityRenderObject::title() will return titleAttr first if that exists (for all elements) accessibilityDescription will return the altAttr for the image Right now, the Mac side relies on the fact that axDescription will give us the alt and title will give us the title for the images (which is why the tests are failing) i think gtk can do the same. First recognize that you're dealing with an image (roleValue==ImageRole), then return the right piece of data

Created attachment 42919 [details] Modified version of Mario's fix: only impacts gtk Please review. Thanks!

(In reply to comment #26) > Created an attachment (id=42919) [details] > Modified version of Mario's fix: only impacts gtk > > Please review. Thanks! Joanie, are you able to run layout tests now? It would be nice if we can cover this with a test or two.

Created attachment 43690 [details] Added a new layout test This patch has the same fix as the patch it is obsoleting. What's new is the addition of a layout test.

Comment on attachment 43690 [details] Added a new layout test I'm a bit confused about the test: are we skipping in on the other platforms because it does not work atm, but it should? Or are we deviating from them here? If it's the latter it should be a GTK+ only test. From comment #25 it seems that the idea was to unify the behavior. (I haven't beet following closely this bug though, so I might be talking nonsense).

(In reply to comment #29) > (From update of attachment 43690 [details]) > I'm a bit confused about the test: are we skipping in on the other platforms > because it does not work atm, but it should? Or are we deviating from them > here? If it's the latter it should be a GTK+ only test. From comment #25 it > seems that the idea was to unify the behavior. (I haven't beet following > closely this bug though, so I might be talking nonsense). My understanding is that what I expect in terms of behavior and what Chris expects in behavior are different and that, therefore, we needed different tests. But then maybe I'm talking nonsense. :-) When I looked at the platform-specific tests, the expected results looked like output from DumpRenderTree (rather than the normal expected results). Please advise, and I will be happy to comply. :-) Thanks!

(In reply to comment #30) > (In reply to comment #29) > > (From update of attachment 43690 [details] [details]) > > I'm a bit confused about the test: are we skipping in on the other platforms > > because it does not work atm, but it should? Or are we deviating from them > > here? If it's the latter it should be a GTK+ only test. From comment #25 it > > seems that the idea was to unify the behavior. (I haven't beet following > > closely this bug though, so I might be talking nonsense). > > My understanding is that what I expect in terms of behavior and what Chris > expects in behavior are different and that, therefore, we needed different > tests. But then maybe I'm talking nonsense. :-) In that case we want GTK only tests, which would go in LayoutTests/platform/gtk > > When I looked at the platform-specific tests, the expected results looked like > output from DumpRenderTree (rather than the normal expected results). Hrm, not sure I understand what you mean, but AFAIK it's up to you if tests are dumped as text or if the whole render tree is dumped. You can configure that through the layoutTestController object. > > Please advise, and I will be happy to comply. :-) Thanks!

(In reply to comment #31) > > When I looked at the platform-specific tests, the expected results looked like > > output from DumpRenderTree (rather than the normal expected results). > > Hrm, not sure I understand what you mean, but AFAIK it's up to you if tests are > dumped as text or if the whole render tree is dumped. You can configure that > through the layoutTestController object. That's because usually tests are the same for all platforms, but platform differences cause the rendering to not match 100%. This specific case is more akin to LayoutTests/platform/gtk/scrollbars/overflow-scrollbar-horizontal-wheel-scroll.html, i.e., the test only makes sense for the platform, as the behavior is specific to it. I would argue that text tests are preferred wherever possible, given that they are not the affected by collateral damage of decisions that change layouting behavior.

Created attachment 43815 [details] proposed fix with platform-specific layout test Thanks for the pointers re the test! Hopefully this is now good to go.

Sorry to be a noodge, but... ping. :-) Thanks!

style-queue successfully ran check-webkit-style on attachment 43815 [details] without any errors

Comment on attachment 43815 [details] proposed fix with platform-specific layout test LGTM.

Comment on attachment 43815 [details] proposed fix with platform-specific layout test Rejecting patch 43815 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1']" exit_code: 1 Last 500 characters of output: ut no file of that name could be found Skipped list contained 'media/media-can-play-ogg.html', but no file of that name could be found Testing 11733 test cases. http/tests/security/cross-frame-access-getOwnPropertyDescriptor.html -> new (results generated in /Users/eseidel/Projects/CommitQueue/LayoutTests/http/tests/security) Exiting early after 1 failures. 8908 tests run. 220.67s total testing time 8907 test cases (99%) succeeded 1 test case (<1%) was new 5 test cases (<1%) had stderr output

I'm assuming that my patch isn't causing the aforementioned test failure. If I'm wrong, I definitely could use a hint.

Comment on attachment 43815 [details] proposed fix with platform-specific layout test I don't think it can be related at all, let's try again.

Comment on attachment 43815 [details] proposed fix with platform-specific layout test Clearing flags on attachment: 43815 Committed r51762: <http://trac.webkit.org/changeset/51762>

All reviewed patches have been landed. Closing bug.

The false rejection was caused by bug 30098.