Bug 101650 – AX: textUnderElement should consider alt text, but skip links and controls

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Dominic Mazzoni]

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).

[James Craig]

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

[James Craig]

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

[Dominic Mazzoni]

Created attachment 178821 [details]
Patch

[Early Warning System Bot]

Comment on attachment 178821 [details]
Patch

Attachment 178821 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15277205

[Early Warning System Bot]

Comment on attachment 178821 [details]
Patch

Attachment 178821 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15276280

[Dominic Mazzoni]

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.

[WebKit Review Bot]

Comment on attachment 178821 [details]
Patch

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

[Build Bot]

Comment on attachment 178821 [details]
Patch

Attachment 178821 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15260485

[chris fleizach]

(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

[Dominic Mazzoni]

Created attachment 178825 [details]
Patch

[chris fleizach]

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

[WebKit Review Bot]

Comment on attachment 178825 [details]
Patch

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

[Early Warning System Bot]

Comment on attachment 178825 [details]
Patch

Attachment 178825 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15260492

[EFL EWS Bot]

Comment on attachment 178821 [details]
Patch

Attachment 178821 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15281131

[Early Warning System Bot]

Comment on attachment 178825 [details]
Patch

Attachment 178825 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15284073

[Build Bot]

Comment on attachment 178821 [details]
Patch

Attachment 178821 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15281130

[EFL EWS Bot]

Comment on attachment 178825 [details]
Patch

Attachment 178825 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15280159

[Peter Beverloo (cr-android ews)]

Comment on attachment 178825 [details]
Patch

Attachment 178825 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15257737

[Build Bot]

Comment on attachment 178825 [details]
Patch

Attachment 178825 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15281137

[WebKit Review Bot]

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.

[kov's GTK+ EWS bot]

Comment on attachment 178825 [details]
Patch

Attachment 178825 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/15258648

[Build Bot]

Comment on attachment 178825 [details]
Patch

Attachment 178825 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15272396

[Peter Beverloo (cr-android ews)]

Comment on attachment 178821 [details]
Patch

Attachment 178821 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15260520

[Dominic Mazzoni]

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

[Dominic Mazzoni]

Created attachment 178868 [details]
Patch

[WebKit Review Bot]

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

[chris fleizach]

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

[Dominic Mazzoni]

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.

[Dominic Mazzoni]

Created attachment 179080 [details]
Patch

[chris fleizach]

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

[Dominic Mazzoni]

Created attachment 179095 [details]
Patch for landing

[WebKit Review Bot]

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

[Dominic Mazzoni]

Created attachment 179113 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 179113 [details]
Patch for landing

Clearing flags on attachment: 179113

Committed r137512: <http://trac.webkit.org/changeset/137512>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[chris fleizach]

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

[Dominic Mazzoni]

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

[Dominic Mazzoni]

Committed r137521: <http://trac.webkit.org/changeset/137521>

[Martin Robinson]

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 ()

[WebKit Review Bot]

Re-opened since this is blocked by bug 104912

[Martin Robinson]

(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+.

[Beth Dakin]

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.

[Dominic Mazzoni]

Created attachment 179795 [details]
Patch

[Dominic Mazzoni]

Created attachment 179796 [details]
Patch

[Dominic Mazzoni]

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?

[WebKit Review Bot]

Comment on attachment 179796 [details]
Patch

Clearing flags on attachment: 179796

Committed r137946: <http://trac.webkit.org/changeset/137946>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.