Closed Bug 1443344 Opened 2 years ago Closed 2 years ago

Correctly propagate failures from child sheets blocked by content policy up to parent sheets

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(1 file, 1 obsolete file)

This testcase:

  <!DOCTYPE html>
  <link rel="stylesheet" href="data:text/css,@import url('http://example.com')"
        onload="alert('success')" onerror="alert('fail')">

when loaded over https:// alerts "success".  That's because css::Loader::LoadStyleLink does a content policy check which fails, but never tells the parent load data about that.

Fixing this on top of bug 1442126 is pretty straightforward.
MozReview-Commit-ID: AArgnuHbCYL
Attachment #8956272 - Flags: review?(bobbyholley)
Comment on attachment 8956272 [details] [diff] [review]
Flag a parent sheet load as failed if an import is blocked by content policy

I guess I still need to write a test.
Attachment #8956272 - Flags: review?(bobbyholley)
MozReview-Commit-ID: AArgnuHbCYL
Attachment #8956326 - Flags: review?(bobbyholley)
Attachment #8956272 - Attachment is obsolete: true
Comment on attachment 8956326 [details] [diff] [review]
Flag a parent sheet load as failed if an import is blocked by content policy

Review of attachment 8956326 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/style/Loader.cpp
@@ +2275,5 @@
>    nsresult rv = CheckContentPolicy(loadingPrincipal, principal, aURL, context, false);
> +  if (NS_WARN_IF(NS_FAILED(rv))) {
> +    if (aParentData) {
> +      aParentData->mLoadFailed = true;
> +    }

Add a comment explaining exactly why we don't need to call MarkLoadTreeFailed here?
Attachment #8956326 - Flags: review?(bobbyholley) → review+
> Add a comment explaining exactly why we don't need to call MarkLoadTreeFailed here?

Nice catch.  We do in fact need MarkLoadTreeFailed.  I added a test that would have caught that and made the change.
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c64f3b38bf58
Flag a parent sheet load as failed if an import is blocked by content policy.  r=bholley
https://hg.mozilla.org/mozilla-central/rev/c64f3b38bf58
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.