Bug 101650

Summary: AX: textUnderElement should consider alt text, but skip links and controls
Product: WebKit Reporter: Dominic Mazzoni <dmazzoni>
Component: AccessibilityAssignee: Dominic Mazzoni <dmazzoni>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, bdakin, cfleizach, dglazkov, gtk-ews, jcraig, jdiggs, mrobinson, peter+ews, philn, webkit-ews, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 104912    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch for landing
none
Patch for landing
none
Patch
none
Patch none

Description Dominic Mazzoni 2012-11-08 15:07:59 PST
This bug made it so that a generic focusable element such as <div tabindex=0> wouldn't be ignored and would get its title from inner text: https://bugs.webkit.org/show_bug.cgi?id=94302

However, this sometimes leads to undesirable behavior, for example:

  <div tabindex=0>
    <a href="#">Alpha</a>
    <a href="#">Bravo</a>
    <a href="#">Charlie</a>
  </div>

In this example, the title of the container is "Alpha Bravo Charlie", which is okay, but VoiceOver will also read "Alpha Bravo Charlie" when you focus each of the links, too. So we shouldn't get the title of a generic focusable control from inner content if it has children that are focusable.

The new proposal is that when an element is focusable and when it doesn't have a specific role, then it should get its title from inner text ONLY if it doesn't have any descendants that are focusable or other containers (like lists).
Comment 1 James Craig 2012-11-08 15:21:26 PST
Probably should also avoid nameFrom:contents on elements with ARIA roles that explicitly disallow this, too. For example, this could get "Foo" as the accessible name:

<div tabindex="0"><div>Foo</div></div>

but this should not:

<div tabindex="0"><div role="group">Foo</div></div>

because the group role defines nameFrom:author (not contents) and the text alternative computation explicitly disallows traversing these nodes for name computation.

http://www.w3.org/WAI/PF/aria/complete#group
Comment 2 James Craig 2012-11-08 15:35:02 PST
You could concatenate any author-supplied name of these, but don't traverse the contents of elements matching these explicit roles.

alert, alertdialog, application, article, banner, combobox, complementary, contentinfo, definition, dialog, document, form, grid, group, list, listbox, log, main, marquee, math, menu, menubar, navigation, note, progressbar, radiogroup, range, region, search, select, separator, scrollbar, slider, spinbutton, status, tablist, tabpanel, textbox, timer, toolbar, tree, treegrid
Comment 3 Dominic Mazzoni 2012-12-11 09:40:49 PST
Created attachment 178821 [details]
Patch
Comment 4 Early Warning System Bot 2012-12-11 09:46:34 PST
Comment on attachment 178821 [details]
Patch

Attachment 178821 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15277205
Comment 5 Early Warning System Bot 2012-12-11 09:47:39 PST
Comment on attachment 178821 [details]
Patch

Attachment 178821 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15276280
Comment 6 Dominic Mazzoni 2012-12-11 09:48:01 PST
OK, here's a first attempt at a patch. It's not quite ready because a few tests have some minor differences in output, mostly whitespace differences.

What do you think of this approach? As I was implementing it I realized that this could help fix another longstanding bug, which is that an element's title, when derived from content, would not use alternative text of descendants. As an example, <button>A picture of a <img src="duck.jpg" alt="duck"></button> should return "A picture of a duck" as its title - and this change fixes that exact case.

This may also help with some cases where using Range to get the text under element was failing because text wasn't rendered.

One consequence of this change is that I was forced to simplify the logic of accessibilityIsIgnored so that it didn't call textUnderElement (indirectly) anymore, because accessibilityIsIgnored is called when an object is deleted, and that was leading to all sorts of crashes. The changes I made to accessibilityIsIgnored have *no* effect on any tests, even though they do far less computation than before - so hopefully you agree that this is a good change. If you like, I could split this into a separate change we could land first.

A potential risk is if using Range to get the plaintext was doing any important text processing that's now lost. If so, it wasn't covered by any tests - but the Range code is pretty complicated and I can't be sure there isn't anything important there.
Comment 7 WebKit Review Bot 2012-12-11 09:48:43 PST
Comment on attachment 178821 [details]
Patch

Attachment 178821 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15258622
Comment 8 Build Bot 2012-12-11 09:49:26 PST
Comment on attachment 178821 [details]
Patch

Attachment 178821 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15260485
Comment 9 chris fleizach 2012-12-11 09:52:22 PST
(In reply to comment #6)
> OK, here's a first attempt at a patch. It's not quite ready because a few tests have some minor differences in output, mostly whitespace differences.
> 
> What do you think of this approach? As I was implementing it I realized that this could help fix another longstanding bug, which is that an element's title, when derived from content, would not use alternative text of descendants. As an example, <button>A picture of a <img src="duck.jpg" alt="duck"></button> should return "A picture of a duck" as its title - and this change fixes that exact case.
> 
> This may also help with some cases where using Range to get the text under element was failing because text wasn't rendered.
> 
> One consequence of this change is that I was forced to simplify the logic of accessibilityIsIgnored so that it didn't call textUnderElement (indirectly) anymore, because accessibilityIsIgnored is called when an object is deleted, and that was leading to all sorts of crashes. The changes I made to accessibilityIsIgnored have *no* effect on any tests, even though they do far less computation than before - so hopefully you agree that this is a good change. If you like, I could split this into a separate change we could land first.
> 

let's make this a separate patch so it will be easier to catch regressions.



> A potential risk is if using Range to get the plaintext was doing any important text processing that's now lost. If so, it wasn't covered by any tests - but the Range code is pretty complicated and I can't be sure there isn't anything important there.

I'm also not sure of what we might lose, but I know what we are losing now (which is the "duck" scenario you mentioned) so I had been thinking for awhile we'd have to do this anyway
Comment 10 Dominic Mazzoni 2012-12-11 10:00:41 PST
Created attachment 178825 [details]
Patch
Comment 11 chris fleizach 2012-12-11 10:02:22 PST
Comment on attachment 178821 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=178821&action=review

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1434
> +    if (!canSetFocusAttribute()) {

what are the kinds of cases you are trying to capture here?

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1445
> +        if (child->isList() || child->isAccessibilityTable() || child->isTree() || child->isCanvas())

how did you determine this list? what about other things like svg roots?

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:625
> +        return AccessibilityNodeObject::textUnderElement();

a AXRenderObject without a renderer is usually an object that is being detached. did you find a case where you needed this call?

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1257
> +    if (!getAttribute(aria_helpAttr).isEmpty() || !getAttribute(aria_describedbyAttr).isEmpty() || !getAttribute(altAttr).isEmpty() || !getAttribute(titleAttr).isEmpty())

is this the exhaustive list of attributes that comprised those three checks (helpText, title, axDescription)?

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1260
> +    if (isGenericFocusableElement() && roleValue() != SVGRootRole && node->firstChild())

this line deserves a comment. why is the node->firstChild() important? what if the first child was aria-hidden or something
Comment 12 WebKit Review Bot 2012-12-11 10:03:30 PST
Comment on attachment 178825 [details]
Patch

Attachment 178825 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15275309
Comment 13 Early Warning System Bot 2012-12-11 10:05:02 PST
Comment on attachment 178825 [details]
Patch

Attachment 178825 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15260492
Comment 14 EFL EWS Bot 2012-12-11 10:05:07 PST
Comment on attachment 178821 [details]
Patch

Attachment 178821 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15281131
Comment 15 Early Warning System Bot 2012-12-11 10:06:26 PST
Comment on attachment 178825 [details]
Patch

Attachment 178825 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15284073
Comment 16 Build Bot 2012-12-11 10:11:57 PST
Comment on attachment 178821 [details]
Patch

Attachment 178821 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15281130
Comment 17 EFL EWS Bot 2012-12-11 10:18:32 PST
Comment on attachment 178825 [details]
Patch

Attachment 178825 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15280159
Comment 18 Peter Beverloo (cr-android ews) 2012-12-11 10:19:05 PST
Comment on attachment 178825 [details]
Patch

Attachment 178825 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15257737
Comment 19 Build Bot 2012-12-11 10:19:46 PST
Comment on attachment 178825 [details]
Patch

Attachment 178825 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15281137
Comment 20 WebKit Review Bot 2012-12-11 10:47:39 PST
Attachment 178821 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/acce..." exit_code: 1
LayoutTests/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:12:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:13:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:14:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 6 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 21 kov's GTK+ EWS bot 2012-12-11 11:05:31 PST
Comment on attachment 178825 [details]
Patch

Attachment 178825 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/15258648
Comment 22 Build Bot 2012-12-11 11:40:05 PST
Comment on attachment 178825 [details]
Patch

Attachment 178825 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15272396
Comment 23 Peter Beverloo (cr-android ews) 2012-12-11 11:54:03 PST
Comment on attachment 178821 [details]
Patch

Attachment 178821 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15260520
Comment 24 Dominic Mazzoni 2012-12-11 13:52:19 PST
Comment on attachment 178821 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=178821&action=review

>> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1434
>> +    if (!canSetFocusAttribute()) {
> 
> what are the kinds of cases you are trying to capture here?

The idea was to check alt text when textUnderElement was called recursively, but not for the top level element that it's initially called on, which is usually focusable. But that isn't quite right - I'll move it inside the loop and do this check on the child directly.

>> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1445
>> +        if (child->isList() || child->isAccessibilityTable() || child->isTree() || child->isCanvas())
> 
> how did you determine this list? what about other things like svg roots?

SVG roots are focusable (not sure why), so that isn't needed.

This list isn't perfect - I was just trying to go from the discussion on this bug, which basically seemed to say that we shouldn't get inner text from a big container element like a list. I added table, tree, and canvas since those seem similar. Almost all of the other cases were focusable.

I moved this into a utility function to make it more clear.

>> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:625
>> +        return AccessibilityNodeObject::textUnderElement();
> 
> a AXRenderObject without a renderer is usually an object that is being detached. did you find a case where you needed this call?

No, you're probably right.

>> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1257
>> +    if (!getAttribute(aria_helpAttr).isEmpty() || !getAttribute(aria_describedbyAttr).isEmpty() || !getAttribute(altAttr).isEmpty() || !getAttribute(titleAttr).isEmpty())
> 
> is this the exhaustive list of attributes that comprised those three checks (helpText, title, axDescription)?

Will respond on this bug, as discussed:

https://bugs.webkit.org/show_bug.cgi?id=104688
Comment 25 Dominic Mazzoni 2012-12-11 13:56:33 PST
Created attachment 178868 [details]
Patch
Comment 26 WebKit Review Bot 2012-12-11 16:00:02 PST
Comment on attachment 178868 [details]
Patch

Attachment 178868 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15284178

New failing tests:
accessibility/aria-toggle-button-with-title.html
Comment 27 chris fleizach 2012-12-11 16:16:34 PST
Comment on attachment 178868 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=178868&action=review

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1429
> +static bool shouldRecurseIntoObjectForInnerText(AccessibilityObject* obj)

what do you think of the name shouldUseAccessiblityObjectInnerText()

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1431
> +    if (obj->canSetFocusAttribute())

some comments about why we shouldn't recurse for canSetFocus() and the other if() check would be good here
Comment 28 Dominic Mazzoni 2012-12-12 10:44:16 PST
Comment on attachment 178868 [details]
Patch

OK, I renamed the bug to capture the essence of the issue and I've got all tests passing, will upload momentarily.

Please take a close look at the test expectations I needed to change. I think most are an improvement, but a couple are unclear. Let me know if you think we should improve the accessible text for a couple of those corner cases or if they look fine.

View in context: https://bugs.webkit.org/attachment.cgi?id=178868&action=review

>> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1429
>> +static bool shouldRecurseIntoObjectForInnerText(AccessibilityObject* obj)
> 
> what do you think of the name shouldUseAccessiblityObjectInnerText()

Sure. I added a lot more comments too.

>> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1431
>> +    if (obj->canSetFocusAttribute())
> 
> some comments about why we shouldn't recurse for canSetFocus() and the other if() check would be good here

Done.
Comment 29 Dominic Mazzoni 2012-12-12 10:54:42 PST
Created attachment 179080 [details]
Patch
Comment 30 chris fleizach 2012-12-12 10:59:20 PST
Comment on attachment 179080 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=179080&action=review

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1482
> +            static_cast<AccessibilityNodeObject*>(child)->alternativeText(textOrder);

you can also use toAccessibilityNode() instead of the static_cast
Comment 31 Dominic Mazzoni 2012-12-12 11:48:41 PST
Created attachment 179095 [details]
Patch for landing
Comment 32 WebKit Review Bot 2012-12-12 11:52:11 PST
Comment on attachment 179095 [details]
Patch for landing

Rejecting attachment 179095 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
ernal-link-anchors2-expected.txt.rej
patching file LayoutTests/platform/mac/accessibility/static-text-role-uses-text-under-element-expected.txt
patching file LayoutTests/platform/mac/accessibility/static-text-role-uses-text-under-element.html
patching file LayoutTests/platform/mac/accessibility/table-with-aria-role-expected.txt
Hunk #1 succeeded at 7 with fuzz 1.

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/15277703
Comment 33 Dominic Mazzoni 2012-12-12 13:09:53 PST
Created attachment 179113 [details]
Patch for landing
Comment 34 WebKit Review Bot 2012-12-12 13:51:53 PST
Comment on attachment 179113 [details]
Patch for landing

Clearing flags on attachment: 179113

Committed r137512: <http://trac.webkit.org/changeset/137512>
Comment 35 WebKit Review Bot 2012-12-12 13:51:59 PST
All reviewed patches have been landed.  Closing bug.
Comment 36 chris fleizach 2012-12-12 14:19:30 PST
i think we might have a test case failure with

accessibility/internal-link-anchors2.html on the mac

-AXTopLevelUIElement: <AXHeading: '[edit] Tourette syndrome'>
+AXTopLevelUIElement: <AXHeading: '[] Tourette syndrome'>
 AXARIABusy: 0
Comment 37 Dominic Mazzoni 2012-12-12 14:24:11 PST
Sorry, I must have merged it incorrectly. I'll do a quick fix for it now.

(In reply to comment #36)
> i think we might have a test case failure with
> 
> accessibility/internal-link-anchors2.html on the mac
> 
> -AXTopLevelUIElement: <AXHeading: '[edit] Tourette syndrome'>
> +AXTopLevelUIElement: <AXHeading: '[] Tourette syndrome'>
>  AXARIABusy: 0
Comment 38 Dominic Mazzoni 2012-12-12 15:06:17 PST
Committed r137521: <http://trac.webkit.org/changeset/137521>
Comment 39 Martin Robinson 2012-12-13 06:50:52 PST
I think this might be causing crashes on the GTK+ bots. After this patch the entire layout test step started timing out and I see this crash locally running tests like accessibility/aria-controls-with-tabs.html.

#0  0x00007ffff61e1e9a in WebCore::AccessibilityRenderObject::nextSibling() const () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#1  0x00007ffff61d4724 in WebCore::AccessibilityNodeObject::textUnderElement() const () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#2  0x00007ffff61e08c1 in WebCore::AccessibilityRenderObject::textUnderElement() const () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#3  0x00007ffff61d48b8 in WebCore::AccessibilityNodeObject::textUnderElement() const () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#4  0x00007ffff61e08c1 in WebCore::AccessibilityRenderObject::textUnderElement() const () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#5  0x00007ffff700d629 in WebCore::AccessibilityObject::accessibilityPlatformIncludesObject() const () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#6  0x00007ffff61e3f10 in WebCore::AccessibilityRenderObject::accessibilityIsIgnoredBase() const () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#7  0x00007ffff61e9626 in WebCore::AccessibilityRenderObject::accessibilityIsIgnored() const () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#8  0x00007ffff61d97df in WebCore::AccessibilityObject::notifyIfIgnoredValueChanged() () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#9  0x00007ffff6922b46 in WebCore::RenderInline::splitFlow(WebCore::RenderObject*, WebCore::RenderBlock*, WebCore::RenderObject*, WebCore::RenderBoxModelObject*) ()
   from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#10 0x00007ffff692307a in WebCore::RenderInline::addChildIgnoringContinuation(WebCore::RenderObject*, WebCore::RenderObject*) ()
   from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#11 0x00007ffff642ad9f in WebCore::NodeRenderingContext::createRendererForElementIfNeeded() () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#12 0x00007ffff63f65e7 in WebCore::Element::createRendererIfNeeded() () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#13 0x00007ffff63ff2f1 in WebCore::Element::attach() () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#14 0x00007ffff63b6273 in WebCore::ContainerNode::attach() () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#15 0x00007ffff63ff35d in WebCore::Element::attach() () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#16 0x00007ffff63fee48 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#17 0x00007ffff63febd8 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#18 0x00007ffff63febd8 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#19 0x00007ffff63febd8 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#20 0x00007ffff63d53fb in WebCore::Document::recalcStyle(WebCore::Node::StyleChange) () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#21 0x00007ffff63d57fe in WebCore::Document::updateStyleIfNeeded() () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#22 0x00007ffff63d6bf4 in WebCore::Document::updateLayout() () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#23 0x00007ffff63d87d9 in WebCore::Document::updateLayoutIgnorePendingStylesheets() () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#24 0x00007ffff63f9f35 in WebCore::Element::focus(bool) () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#25 0x00007ffff6bc9dfe in WebCore::jsElementPrototypeFunctionFocus(JSC::ExecState*) () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#26 0x00007fffa864d265 in ?? ()
#27 0xffff000000000002 in ?? ()
#28 0x00007ffff7b7ccd3 in llint_op_call () from /home/martin/WebKit/WebKitBuild/Release/.libs/libjavascriptcoregtk-3.0.so.0
#29 0x00007fffa7a20000 in ?? ()
#30 0x00007ffff7c199a1 in JSC::ProgramExecutable::compileInternal(JSC::ExecState*, JSC::JSScope*, JSC::JITCode::JITType, unsigned int) ()
   from /home/martin/WebKit/WebKitBuild/Release/.libs/libjavascriptcoregtk-3.0.so.0
#31 0x00007ffff7b18744 in JSC::Interpreter::execute(JSC::ProgramExecutable*, JSC::ExecState*, JSC::JSObject*) () from /home/martin/WebKit/WebKitBuild/Release/.libs/libjavascriptcoregtk-3.0.so.0
#32 0x00007ffff7c0a38e in JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, JSC::JSValue*) () from /home/martin/WebKit/WebKitBuild/Release/.libs/libjavascriptcoregtk-3.0.so.0
#33 0x00007ffff6261395 in WebCore::ScriptController::evaluateInWorld(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld*) ()
   from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#34 0x00007ffff62619f2 in WebCore::ScriptController::evaluate(WebCore::ScriptSourceCode const&) () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
---Type <return> to continue, or q <return> to quit---#35 0x00007ffff643f4f3 in WebCore::ScriptElement::executeScript(WebCore::ScriptSourceCode const&) () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#36 0x00007ffff6443503 in WebCore::ScriptElement::prepareScript(WTF::TextPosition const&, WebCore::ScriptElement::LegacyTypeSupport) ()
   from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#37 0x00007ffff65faafb in WebCore::HTMLScriptRunner::runScript(WebCore::Element*, WTF::TextPosition const&) () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#38 0x00007ffff65fb4dd in WebCore::HTMLScriptRunner::execute(WTF::PassRefPtr<WebCore::Element>, WTF::TextPosition const&) ()
   from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#39 0x00007ffff65e340e in WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder() () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#40 0x00007ffff65e34d2 in WebCore::HTMLDocumentParser::canTakeNextToken(WebCore::HTMLDocumentParser::SynchronousMode, WebCore::PumpSession&) ()
   from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#41 0x00007ffff65e36b0 in WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode) () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#42 0x00007ffff65e55e1 in WebCore::HTMLDocumentParser::resumeParsingAfterScriptExecution() () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#43 0x00007ffff65e58d8 in WebCore::HTMLDocumentParser::notifyFinished(WebCore::CachedResource*) () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#44 0x00007ffff673a8fd in WebCore::CachedResource::checkNotify() () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#45 0x00007ffff67a7b99 in WebCore::SubresourceLoader::didFinishLoading(double) () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#46 0x00007ffff6ed86f2 in WebCore::readCallback(_GObject*, _GAsyncResult*, void*) () from /home/martin/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-3.0.so.0
#47 0x00007ffff542b234 in async_ready_callback_wrapper (source_object=0xc9b640, res=0xc9ba70, user_data=0x7fffe8667690) at ginputstream.c:529
#48 0x00007ffff544001e in g_simple_async_result_complete (simple=0xc9ba70) at gsimpleasyncresult.c:777
#49 0x00007ffff54400b8 in complete_in_idle_cb_for_thread (_data=0xca1c80) at gsimpleasyncresult.c:845
#50 0x00007ffff7ec8575 in g_main_dispatch (context=0x4d50c0) at gmain.c:2784
#51 g_main_context_dispatch (context=context@entry=0x4d50c0) at gmain.c:3288
#52 0x00007ffff7ec88b8 in g_main_context_iterate (context=0x4d50c0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3359
#53 0x00007ffff7ec8d22 in g_main_loop_run (loop=0x5bfc30) at gmain.c:3553
#54 0x00007ffff5a2e9a5 in gtk_main () at gtkmain.c:1161
#55 0x000000000043a075 in runTest(std::string const&) ()
#56 0x000000000043ab99 in main ()
Comment 40 WebKit Review Bot 2012-12-13 07:02:05 PST
Re-opened since this is blocked by bug 104912
Comment 41 Martin Robinson 2012-12-13 07:04:41 PST
(In reply to comment #40)
> Re-opened since this is blocked by bug 104912

Normally I would try to contact you guys, but I'm in the wrong timezone and the bots are scattered to the wind like so many grains of sand. I'm going to rollout the patch now, but let me know if you need a hand fixing things for GTK+.
Comment 42 Beth Dakin 2012-12-13 12:51:18 PST
So this was rolled out with http://trac.webkit.org/changeset/137592 , and the rollout caused accessibility/internal-link-anchors2.html to start failing. I just updated the results. Just wanted to note that here.
Comment 43 Dominic Mazzoni 2012-12-17 13:41:35 PST
Created attachment 179795 [details]
Patch
Comment 44 Dominic Mazzoni 2012-12-17 13:47:43 PST
Created attachment 179796 [details]
Patch
Comment 45 Dominic Mazzoni 2012-12-17 13:48:09 PST
As discussed offline with GTK folks, this patch now restores the previous behavior for GTK and skips the two affected tests. I filed a new bug (https://bugs.webkit.org/show_bug.cgi?id=105214) to track the work needed on the GTK side.

Take another quick look?
Comment 46 WebKit Review Bot 2012-12-17 14:25:08 PST
Comment on attachment 179796 [details]
Patch

Clearing flags on attachment: 179796

Committed r137946: <http://trac.webkit.org/changeset/137946>
Comment 47 WebKit Review Bot 2012-12-17 14:25:15 PST
All reviewed patches have been landed.  Closing bug.