Bug 148615 – http/tests/w3c/dom/nodes/Element-matches.html is flaky

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Chris Dumez]

http/tests/w3c/dom/nodes/Element-matches.html is flaky:
--- /Volumes/Data/slave/yosemite-debug-tests-wk1/build/layout-test-results/http/tests/w3c/dom/nodes/Element-matches-expected.txt
+++ /Volumes/Data/slave/yosemite-debug-tests-wk1/build/layout-test-results/http/tests/w3c/dom/nodes/Element-matches-actual.txt
@@ -171,7 +171,7 @@
 PASS In-document Element.matches: :empty pseudo-class selector, matching all empty elements (with no refNodes): #pseudo-empty :empty 
 PASS In-document Element.matches: :link and :visited pseudo-class selectors, matching a and area elements with href attributes (with no refNodes): #pseudo-link :link, #pseudo-link :visited 
 PASS In-document Element.matches: :link and :visited pseudo-class selectors, matching link elements with href attributes (with no refNodes): #head :link, #head :visited 
-PASS In-document Element.matches: :target pseudo-class selector, matching the element referenced by the URL fragment identifier (with no refNodes): :target 
+FAIL In-document Element.matches: :target pseudo-class selector, matching the element referenced by the URL fragment identifier (with no refNodes): :target assert_true: The element #target should match the selector. expected true got false
 PASS In-document Element.matches: :lang pseudo-class selector, matching inherited language (with no refNodes): #pseudo-lang-div1:lang(en) 
 PASS In-document Element.matches: :lang pseudo-class selector, matching specified language with exact value (with no refNodes): #pseudo-lang-div2:lang(fr) 
 PASS In-document Element.matches: :lang pseudo-class selector, matching specified language with partial value (with no refNodes): #pseudo-lang-div3:lang(en) 

When the test is run on its own, that check seems to pass. However, when running all the layout tests, the check is failing. This seems to indicate we have a bug in WebCore, possibly related to caching?

[Chris Dumez]

I haven't marked the test as flaky yet. For now, I have merely rebaselined it in <https://trac.webkit.org/r189158>.

[Chris Dumez]

(In reply to comment #1)
> I haven't marked the test as flaky yet. For now, I have merely rebaselined
> it in <https://trac.webkit.org/r189158>.

Actually, I will have to mark it as flaky because it seems bots give different results:
https://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK2%20(Tests)/r189158%20(14158)/http/tests/w3c/dom/nodes/Element-matches-diff.txt

[Chris Dumez]

(In reply to comment #2)
> (In reply to comment #1)
> > I haven't marked the test as flaky yet. For now, I have merely rebaselined
> > it in <https://trac.webkit.org/r189158>.
> 
> Actually, I will have to mark it as flaky because it seems bots give
> different results:
> https://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK2%20(Tests)/
> r189158%20(14158)/http/tests/w3c/dom/nodes/Element-matches-diff.txt

Marked as flaky in <http://trac.webkit.org/changeset/189159>.

[Chris Dumez]

FrameView::scrollToAnchor() also has logic to delay the scroll until the stylesheets are loaded. Which delays the cssTarget() update on the Document as well:

bool FrameView::scrollToAnchor(const String& name)
{
    ASSERT(frame().document());
    auto& document = *frame().document();

    if (!document.haveStylesheetsLoaded()) {
        document.setGotoAnchorNeededAfterStylesheetsLoad(true);
        return false;
    }

    document.setGotoAnchorNeededAfterStylesheetsLoad(false);

    Element* anchorElement = document.findAnchor(name);

    // Setting to null will clear the current target.
    document.setCSSTarget(anchorElement);

[Chris Dumez]

*** Bug 148638 has been marked as a duplicate of this bug. ***

[Chris Dumez]

Created attachment 260316 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 260316 [details]
Patch

Nice catch!

[WebKit Commit Bot]

Comment on attachment 260316 [details]
Patch

Clearing flags on attachment: 260316

Committed r189198: <http://trac.webkit.org/changeset/189198>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

This broke http/tests/navigation/anchor-frames-same-origin.html on WebKit1:

-PASS document.body.scrollTop > 0 is true
+FAIL document.body.scrollTop > 0 should be true. Was false.
 PASS document.body.scrollLeft == 0 is true
 PASS successfullyParsed is true

[Chris Dumez]

(In reply to comment #10)
> This broke http/tests/navigation/anchor-frames-same-origin.html on WebKit1:
> 
> -PASS document.body.scrollTop > 0 is true
> +FAIL document.body.scrollTop > 0 should be true. Was false.
>  PASS document.body.scrollLeft == 0 is true
>  PASS successfullyParsed is true

Hmm, Interesting that the EWS did not see this. I am looking now.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 148670

[Chris Dumez]

Created attachment 260407 [details]
Patch

[Chris Dumez]

Created attachment 260409 [details]
Patch

[Chris Dumez]

Comment on attachment 260409 [details]
Patch

Clearing flags on attachment: 260409

Committed r189252: <http://trac.webkit.org/changeset/189252>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

rdar://problem/22544150