New issue
Advanced search Search tips

Issue 903785 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 26
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

@imports in HTML missing quote marks are mistakenly hidden from the Preload Scanner

Reported by csswizar...@gmail.com, Nov 9

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.77 Safari/537.36

Example URL:
https://csswizardry.com/demos/2018/11/import/blocked-by-css.html?

Steps to reproduce the problem:
1. Have a page with a synchronous `<link rel="stylesheet" />` and/or scnchronous `<script src="">` BEFORE an in-page `@import url(style.css)` that omits the optional quote marks.
2. Reload page with Network panel open
3. Note lack of parallelisation: files are serialised
4. Edit `@import url(style.css)` to be `@import url("style.css")` (note addition of quote marks)
5. Refresh and observe waterfall
6. Parallelisation!

What is the expected behavior?
I would expect the Preload Scanner to identify the IMPORT regardless of the presence of quote marks

What went wrong?
The Preload Scanner isn’t noticing @import URLs that omit OPTIONAL quotes

Did this work before? N/A 

Chrome version: 70.0.3538.77  Channel: stable
OS Version: OS X 10.13.6
Flash Version:
 
Owner: yoavweiss@chromium.org
Cc: domfarolino@gmail.com
Labels: Needs-Triage-M70
Status: Started (was: Unconfirmed)
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 26

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

commit 6eb116c494d620ced1f82156eddf91aeca6bba7d
Author: Yoav Weiss <yoavweiss@chromium.org>
Date: Mon Nov 26 00:08:19 2018

Fix CSSPreloadScanner to avoid missing rules

This fixes a couple of bugs with the CSSPReloadScanner that made it miss
rules in which the URL wasn't quoted or the rule didn't end with a
semicolon. It also adds tentative WPT tests for that functionality, as
the tests are also relevant for WebKit.

Bug:  903785 
Change-Id: I6d7dae1db3617184148fd71dfdb62fa1ee19e74f
Reviewed-on: https://chromium-review.googlesource.com/c/1331042
Commit-Queue: Yoav Weiss <yoavweiss@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610744}
[add] https://crrev.com/6eb116c494d620ced1f82156eddf91aeca6bba7d/third_party/WebKit/LayoutTests/external/wpt/loading/preloader-css-import-no-quote.tentative.html
[add] https://crrev.com/6eb116c494d620ced1f82156eddf91aeca6bba7d/third_party/WebKit/LayoutTests/external/wpt/loading/preloader-css-import-no-semicolon.tentative.html
[add] https://crrev.com/6eb116c494d620ced1f82156eddf91aeca6bba7d/third_party/WebKit/LayoutTests/external/wpt/loading/preloader-css-import-no-space.tentative.html
[add] https://crrev.com/6eb116c494d620ced1f82156eddf91aeca6bba7d/third_party/WebKit/LayoutTests/external/wpt/loading/preloader-css-import-single-quote.tentative.html
[add] https://crrev.com/6eb116c494d620ced1f82156eddf91aeca6bba7d/third_party/WebKit/LayoutTests/external/wpt/loading/preloader-css-import.tentative.html
[add] https://crrev.com/6eb116c494d620ced1f82156eddf91aeca6bba7d/third_party/WebKit/LayoutTests/external/wpt/loading/resources/dummy.css
[modify] https://crrev.com/6eb116c494d620ced1f82156eddf91aeca6bba7d/third_party/blink/renderer/core/html/parser/css_preload_scanner.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 26

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

commit 0af8bdb3744e16012d3a6450a7ba272a9d503ac5
Author: Kent Tamura <tkent@chromium.org>
Date: Mon Nov 26 02:09:14 2018

Revert "Fix CSSPreloadScanner to avoid missing rules"

This reverts commit 6eb116c494d620ced1f82156eddf91aeca6bba7d.

Reason for revert: The CL added files to non-existent WebKit/LayoutTests/.

Original change's description:
> Fix CSSPreloadScanner to avoid missing rules
> 
> This fixes a couple of bugs with the CSSPReloadScanner that made it miss
> rules in which the URL wasn't quoted or the rule didn't end with a
> semicolon. It also adds tentative WPT tests for that functionality, as
> the tests are also relevant for WebKit.
> 
> Bug:  903785 
> Change-Id: I6d7dae1db3617184148fd71dfdb62fa1ee19e74f
> Reviewed-on: https://chromium-review.googlesource.com/c/1331042
> Commit-Queue: Yoav Weiss <yoavweiss@chromium.org>
> Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#610744}

TBR=kouhei@chromium.org,csharrison@chromium.org,yoavweiss@chromium.org

Change-Id: Ib2dcae03345d0f05f970b226a727f28e11a6c866
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  903785 
Reviewed-on: https://chromium-review.googlesource.com/c/1350014
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610749}
[delete] https://crrev.com/8100d4b5a4f5b6f4a6957acd57a1b167022c5cae/third_party/WebKit/LayoutTests/external/wpt/loading/preloader-css-import-no-quote.tentative.html
[delete] https://crrev.com/8100d4b5a4f5b6f4a6957acd57a1b167022c5cae/third_party/WebKit/LayoutTests/external/wpt/loading/preloader-css-import-no-semicolon.tentative.html
[delete] https://crrev.com/8100d4b5a4f5b6f4a6957acd57a1b167022c5cae/third_party/WebKit/LayoutTests/external/wpt/loading/preloader-css-import-no-space.tentative.html
[delete] https://crrev.com/8100d4b5a4f5b6f4a6957acd57a1b167022c5cae/third_party/WebKit/LayoutTests/external/wpt/loading/preloader-css-import-single-quote.tentative.html
[delete] https://crrev.com/8100d4b5a4f5b6f4a6957acd57a1b167022c5cae/third_party/WebKit/LayoutTests/external/wpt/loading/preloader-css-import.tentative.html
[delete] https://crrev.com/8100d4b5a4f5b6f4a6957acd57a1b167022c5cae/third_party/WebKit/LayoutTests/external/wpt/loading/resources/dummy.css
[modify] https://crrev.com/0af8bdb3744e16012d3a6450a7ba272a9d503ac5/third_party/blink/renderer/core/html/parser/css_preload_scanner.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 26

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

commit 30ecdaf63da0c5e1d81caf93c9836ec5564d9647
Author: Yoav Weiss <yoavweiss@chromium.org>
Date: Mon Nov 26 09:56:22 2018

Fix CSSPreloadScanner to avoid missing rules (reland)

This fixes a couple of bugs with the CSSPReloadScanner that made it miss
rules in which the URL wasn't quoted or the rule didn't end with a
semicolon. It also adds tentative WPT tests for that functionality, as
the tests are also relevant for WebKit.

This is a reland of
https://chromium-review.googlesource.com/c/chromium/src/+/1331042

Bug:  903785 
Change-Id: I401c252a42fbb96dee9c7942e0a4f8d5b6850244
TBR: kouhei
Reviewed-on: https://chromium-review.googlesource.com/c/1349980
Reviewed-by: Yoav Weiss <yoav@yoav.ws>
Commit-Queue: Yoav Weiss <yoav@yoav.ws>
Cr-Commit-Position: refs/heads/master@{#610796}
[modify] https://crrev.com/30ecdaf63da0c5e1d81caf93c9836ec5564d9647/third_party/blink/renderer/core/html/parser/css_preload_scanner.cc
[add] https://crrev.com/30ecdaf63da0c5e1d81caf93c9836ec5564d9647/third_party/blink/web_tests/external/wpt/loading/preloader-css-import-no-quote.tentative.html
[add] https://crrev.com/30ecdaf63da0c5e1d81caf93c9836ec5564d9647/third_party/blink/web_tests/external/wpt/loading/preloader-css-import-no-semicolon.tentative.html
[add] https://crrev.com/30ecdaf63da0c5e1d81caf93c9836ec5564d9647/third_party/blink/web_tests/external/wpt/loading/preloader-css-import-no-space.tentative.html
[add] https://crrev.com/30ecdaf63da0c5e1d81caf93c9836ec5564d9647/third_party/blink/web_tests/external/wpt/loading/preloader-css-import-single-quote.tentative.html
[add] https://crrev.com/30ecdaf63da0c5e1d81caf93c9836ec5564d9647/third_party/blink/web_tests/external/wpt/loading/preloader-css-import.tentative.html
[add] https://crrev.com/30ecdaf63da0c5e1d81caf93c9836ec5564d9647/third_party/blink/web_tests/external/wpt/loading/resources/dummy.css

Status: Fixed (was: Started)

Sign in to add a comment