New issue
Advanced search Search tips

Issue 843523 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

document.querySelector(":target") is flaky in window.load event.

Project Member Reported by arthurso...@chromium.org, May 16 2018

Issue description

Depending on which one of this function is called first:
* Document::ImplicitClose()
* Document::ProcessFragment()

document.querySelector(":target") may return null or not.

Regression test: https://chromium-review.googlesource.com/c/chromium/src/+/1061456
Probable solution: https://chromium-review.googlesource.com/c/chromium/src/+/1057507
 
i need to have access to this because ==_[input-read] is my code for that nico cold.
download.jpeg
14.4 KB View Download
Project Member

Comment 3 by bugdroid1@chromium.org, May 25 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cb7ef37d1490ce7eb9c22701f4ff9a4759cdab09

commit cb7ef37d1490ce7eb9c22701f4ff9a4759cdab09
Author: arthursonzogni <arthursonzogni@chromium.org>
Date: Fri May 25 16:58:06 2018

Add regression test for  https://crbug.com/843523 .

Test that document.querySelector(":target") works when called in the
window.load event.

This test is a subset of dom/nodes/Element-matches.html, the difference
is that this test is more lightweight. It causes the window.load event
to be sent earlier. There is a race condition. Currently, it will
only work when FrameLoader::ProcessFragment is called before
Document::ImplicitClose().

Probable fix:
https://chromium-review.googlesource.com/c/chromium/src/+/1057507

Bug:  843523 
Change-Id: Ibf7b0d7621b18c3dfefaaa323bbd335d86ce0652
Reviewed-on: https://chromium-review.googlesource.com/1061456
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561916}
[modify] https://crrev.com/cb7ef37d1490ce7eb9c22701f4ff9a4759cdab09/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/cb7ef37d1490ce7eb9c22701f4ff9a4759cdab09/third_party/WebKit/LayoutTests/external/wpt/dom/nodes/query-target-in-load-event.html
[add] https://crrev.com/cb7ef37d1490ce7eb9c22701f4ff9a4759cdab09/third_party/WebKit/LayoutTests/external/wpt/dom/nodes/query-target-in-load-event.part.html

Project Member

Comment 4 by bugdroid1@chromium.org, May 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2d1846bed03be552223791562288aea46c7c0c8a

commit 2d1846bed03be552223791562288aea46c7c0c8a
Author: arthursonzogni <arthursonzogni@chromium.org>
Date: Mon May 28 12:28:02 2018

Ensure CSS :target is set before calling the load event.

I am working on https://crbug.com/831155. It causes the frame to commit
a navigation more quickly. As a result, some tests that are relying on
how things are scheduled in blink are failling.

This CL makes sure at least two WPT tests keep working:
* external/wpt/dom/nodes/Element-matches.html
* external/wpt/dom/nodes/Element-webkitMatchesSelector.html

What happens in theses tests:
In the frame.load event, document.querySelector(":target") is used.
When an URL has an anchor, like in http://example.com#anchor, it returns
the element with the "anchor" ID.

The problem is that blink::Document::SetCSSTarget(Element*) may
currently be called after the 'load' event is executed.

The solution is simply to exchange two lines:
1) The one that will maybe send the 'load' event.
   frame_->GetDocument()->CheckCompleted();
2) The one that will call Document::SetCSSTarget()
   ProcessFragment([...])

This CL is similar to:
https://chromium-review.googlesource.com/c/chromium/src/+/1042191
It is the same solutioa to a different problem.

This test is disabled because of  http://crbug.com/843192 
* svg/animations/viewspec-checkaspectparams.html

Bug: 831155,  843523 ,  843192 
Change-Id: I8ecedd212d3ce30d252ec9c1b1f22214f7bc012a
Reviewed-on: https://chromium-review.googlesource.com/1057507
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562235}
[modify] https://crrev.com/2d1846bed03be552223791562288aea46c7c0c8a/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/2d1846bed03be552223791562288aea46c7c0c8a/third_party/blink/renderer/core/loader/frame_loader.cc

Status: Fixed (was: Started)

Sign in to add a comment