Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable @import in replace() and replaceSync() #22374

Merged
merged 1 commit into from
Apr 7, 2020

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Mar 21, 2020

The CSSWG resolved [1] that "@import doesn't parse in constructable
stylesheets and as a result throw syntax errors". We have a comment [2]
requesting clarification, but the working assumption is:

  • calling replace() with text that includes @import will ignore the
    @import, and issue a console warning.
  • calling replaceSync() with text that includes @import will ignore
    the @import, and issue a console warning.
  • calling insertRule() on a constructed stylesheet with an @import
    rule will throw a SyntaxError. (no change here)

Prior to this CL, the behavior of Chromium was:

  • calling replace() with text that includes @import would work. The
    returned Promise would resolve once all @imports were loaded. Any
    invalid @import rules would cause a NetworkError to be thrown.
  • calling replaceSync() with text that includes @import would throw
    a "NotAllowed" exception.
  • calling insertRule() on a constructed stylesheet with an @import
    rule will throw a SyntaxError.

The Intent to Remove for this change is at [3].

[1] WICG/construct-stylesheets#119 (comment)
[2] WICG/construct-stylesheets#119 (comment)
[3] https://groups.google.com/a/chromium.org/g/blink-dev/c/RKG8oxp22RY/m/fdFnG1rGCgAJ

Bug: 1055943
Change-Id: I0a8444289b137b4c2880be5231696bb526331107
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2106996
Reviewed-by: Mason Freed <masonfreed@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756894}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Chromium project.

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-2106996 branch 5 times, most recently from 7400801 to 641918a Compare March 26, 2020 17:01
The CSSWG resolved [1] that "@import doesn't parse in constructable
stylesheets and as a result throw syntax errors". We have a comment [2]
requesting clarification, but the working assumption is:

- calling replace() with text that includes @import will ignore the
  @import, and issue a console warning.
- calling replaceSync() with text that includes @import will ignore
  the @import, and issue a console warning.
- calling insertRule() on a constructed stylesheet with an @import
  rule will throw a SyntaxError. (no change here)

*Prior* to this CL, the behavior of Chromium was:
- calling replace() with text that includes @import would work. The
  returned Promise would resolve once all @imports were loaded. Any
  invalid @import rules would cause a NetworkError to be thrown.
- calling replaceSync() with text that includes @import would throw
  a "NotAllowed" exception.
- calling insertRule() on a constructed stylesheet with an @import
  rule will throw a SyntaxError.

The Intent to Remove for this change is at [3].

[1] WICG/construct-stylesheets#119 (comment)
[2] WICG/construct-stylesheets#119 (comment)
[3] https://groups.google.com/a/chromium.org/g/blink-dev/c/RKG8oxp22RY/m/fdFnG1rGCgAJ

Bug: 1055943
Change-Id: I0a8444289b137b4c2880be5231696bb526331107
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2106996
Reviewed-by: Mason Freed <masonfreed@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756894}
@chromium-wpt-export-bot chromium-wpt-export-bot merged commit e797def into master Apr 7, 2020
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-2106996 branch April 7, 2020 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants