WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 25524
[Gtk] Expose the title attribute to assistive technologies
https://bugs.webkit.org/show_bug.cgi?id=25524
Summary
[Gtk] Expose the title attribute to assistive technologies
Joanmarie Diggs
Reported
2009-05-03 12:45:37 PDT
According to the W3C Recommendations, the title attribute (
http://www.w3.org/TR/html401/struct/global.html#title
) "offers advisory information about the element for which it is set." WebKit-gtk presents this information to the user via a tooltip which appears when the user hovers the mouse over the element in question; however, neither the presence of the title attribute nor its content is exposed as a property of the accessible object to which it pertains. While "title" suggests that this information might best be exposed as the name of the accessible object, there are a number of reasons why this information might be better suited for exposure through some other means: 1. The W3C Recommendations describe the title attribute as "advisory information," which suggests that both in function and potentially in length, this is not the object's name. 2. The title attribute can be applied to many objects, some of which might already have -- or should have -- accessible names (such as a push button, pending the fix for
bug 25523
). There needs to be a way to expose both a name and a title, should both be present. 3. Ideally, an assistive technology -- and users thereof -- seeking to obtain the title associated with a particular accessible, should have a reliable, consistent means by which to obtain that information. Therefore, it is recommended that the content of the title attribute be exposed as the object's accessible description.
Attachments
test case
(607 bytes, text/html)
2009-05-03 12:55 PDT
,
Joanmarie Diggs
no flags
Details
Patch proposal
(3.43 KB, patch)
2009-07-17 00:56 PDT
,
Mario Sanchez Prada
no flags
Details
Formatted Diff
Diff
Patch proposal, with ChangeLog
(3.77 KB, patch)
2009-07-17 05:34 PDT
,
Mario Sanchez Prada
gustavo
: review-
Details
Formatted Diff
Diff
Patch proposal, indentation fixed
(3.83 KB, patch)
2009-07-17 09:12 PDT
,
Mario Sanchez Prada
eric
: review-
Details
Formatted Diff
Diff
Modified version of Mario's fix: only impacts gtk
(4.92 KB, patch)
2009-11-10 20:14 PST
,
Joanmarie Diggs
no flags
Details
Formatted Diff
Diff
Added a new layout test
(10.01 KB, patch)
2009-11-22 16:13 PST
,
Joanmarie Diggs
no flags
Details
Formatted Diff
Diff
proposed fix with platform-specific layout test
(8.75 KB, patch)
2009-11-24 15:12 PST
,
Joanmarie Diggs
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Joanmarie Diggs
Comment 1
2009-05-03 12:55:52 PDT
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?
Joanmarie Diggs
Comment 2
2009-05-03 12:58:21 PDT
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!
Mario Sanchez Prada
Comment 3
2009-07-16 14:47:43 PDT
(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.
Joanmarie Diggs
Comment 4
2009-07-16 15:15:16 PDT
(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
Mario Sanchez Prada
Comment 5
2009-07-16 23:24:28 PDT
(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.
Mario Sanchez Prada
Comment 6
2009-07-17 00:56:06 PDT
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.
Mario Sanchez Prada
Comment 7
2009-07-17 05:34:01 PDT
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.
Gustavo Noronha (kov)
Comment 8
2009-07-17 07:47:16 PDT
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!
Mario Sanchez Prada
Comment 9
2009-07-17 09:12:16 PDT
Created
attachment 32944
[details]
Patch proposal, indentation fixed Fixed indentation problems
Gustavo Noronha (kov)
Comment 10
2009-07-17 09:24:13 PDT
Comment on
attachment 32944
[details]
Patch proposal, indentation fixed r=me
Gustavo Noronha (kov)
Comment 11
2009-07-17 09:49:03 PDT
Landed as
r46038
.
Eric Seidel (no email)
Comment 12
2009-07-17 13:42:04 PDT
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.
Jan Alonzo
Comment 13
2009-07-17 14:29:02 PDT
(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
Jan Alonzo
Comment 14
2009-07-17 20:23:48 PDT
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----------------------------
Joanmarie Diggs
Comment 15
2009-07-18 19:36:36 PDT
(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.
Mario Sanchez Prada
Comment 16
2009-07-23 02:02:13 PDT
(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.
Joanmarie Diggs
Comment 17
2009-07-23 06:01:12 PDT
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.
Mario Sanchez Prada
Comment 18
2009-07-23 07:02:40 PDT
(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?
Joanmarie Diggs
Comment 19
2009-07-23 08:00:28 PDT
(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!
Mario Sanchez Prada
Comment 20
2009-07-23 08:12:10 PDT
(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!
Xan Lopez
Comment 21
2009-07-23 10:11:50 PDT
(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.
Gustavo Noronha (kov)
Comment 22
2009-08-12 18:36:41 PDT
(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.
Joanmarie Diggs
Comment 23
2009-10-17 19:18:10 PDT
Ping. (Where are we with this?)
Joanmarie Diggs
Comment 24
2009-10-26 14:58:24 PDT
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!!
chris fleizach
Comment 25
2009-10-27 23:12:40 PDT
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
Joanmarie Diggs
Comment 26
2009-11-10 20:14:16 PST
Created
attachment 42919
[details]
Modified version of Mario's fix: only impacts gtk Please review. Thanks!
Jan Alonzo
Comment 27
2009-11-11 03:13:30 PST
(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.
Joanmarie Diggs
Comment 28
2009-11-22 16:13:19 PST
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.
Xan Lopez
Comment 29
2009-11-24 09:14:01 PST
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).
Joanmarie Diggs
Comment 30
2009-11-24 09:23:40 PST
(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!
Xan Lopez
Comment 31
2009-11-24 09:47:53 PST
(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!
Gustavo Noronha (kov)
Comment 32
2009-11-24 11:16:33 PST
(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.
Joanmarie Diggs
Comment 33
2009-11-24 15:12:47 PST
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.
Joanmarie Diggs
Comment 34
2009-11-29 21:10:24 PST
Sorry to be a noodge, but... ping. :-) Thanks!
Adam Barth
Comment 35
2009-11-30 12:38:50 PST
style-queue successfully ran check-webkit-style on
attachment 43815
[details]
without any errors
Xan Lopez
Comment 36
2009-12-07 05:23:49 PST
Comment on
attachment 43815
[details]
proposed fix with platform-specific layout test LGTM.
WebKit Commit Bot
Comment 37
2009-12-07 05:36:23 PST
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
Joanmarie Diggs
Comment 38
2009-12-07 05:42:30 PST
I'm assuming that my patch isn't causing the aforementioned test failure. If I'm wrong, I definitely could use a hint.
Xan Lopez
Comment 39
2009-12-07 06:13:37 PST
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.
WebKit Commit Bot
Comment 40
2009-12-07 06:27:52 PST
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
>
WebKit Commit Bot
Comment 41
2009-12-07 06:28:00 PST
All reviewed patches have been landed. Closing bug.
Eric Seidel (no email)
Comment 42
2009-12-07 11:31:37 PST
The false rejection was caused by
bug 30098
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug