Closed
Bug 1431814
Opened 6 years ago
Closed 6 years ago
blob URL iframes should inherit parent's controlling service worker
Categories
(Core :: DOM: Service Workers, enhancement, P2)
Core
DOM: Service Workers
Tracking
()
RESOLVED
FIXED
mozilla60
Tracking | Status | |
---|---|---|
firefox60 | --- | fixed |
People
(Reporter: bkelly, Assigned: bkelly)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 1 obsolete file)
2.35 KB,
patch
|
asuth
:
review+
|
Details | Diff | Splinter Review |
15.20 KB,
patch
|
bkelly
:
review+
|
Details | Diff | Splinter Review |
While fixing blob URL workers to inherit their parent's service worker controller in bug 1231211 I noticed that iframes don't do the inheritance either. Filing this as a follow-up bug since I'm not sure where to do it yet. Also, we should add a WPT test for this.
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 1•6 years ago
|
||
This fixes it in local testing. I need to add a WPT for it.
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
Updated•6 years ago
|
Priority: -- → P2
Assignee | ||
Comment 2•6 years ago
|
||
Assignee | ||
Comment 3•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=75592881c7b025a9bfcb9f07843f1345efd8b763
Assignee | ||
Comment 4•6 years ago
|
||
Comment on attachment 8944062 [details] [diff] [review] P1 Allow the controller to be inheritted for other local URLs like blob: and data:. r=asuth This fixes service worker controller inheritance for blob: URLs. The fix is very similar to what we did in bug 1426979. Basically we just have to ignore the LoadInfo for "local" URL schemes so we can properly inherit. Local URLs are defined in the spec as: A local scheme is a scheme that is "about", "blob", "data", or "filesystem". https://fetch.spec.whatwg.org/#local-scheme I don't think we have filesystem: URLs, so I just made this work for about, blob, and data. I explicitly limit about: to things in the spec as I don't know that we want to inherit for other firefox specific about: pages. Note, this change also isn't strictly necessary for data URL frames. It seems that worked before anyway. Possibly because they don't have LoadInfo objects or something. I decided to include them here for completeness, though, since the local URL definition in the spec includes them.
Attachment #8944062 -
Flags: review?(bugmail)
Assignee | ||
Comment 5•6 years ago
|
||
Comment on attachment 8944481 [details] [diff] [review] P2 Add blob-url-inherits-controller.https.html wpt test. r=asuth Andrew, this adds a WPT test that checks for controller inheritence for blob URL frames and workers. Currently we fail the worker test because we don't expose navigator.serviceWorker on worker globals yet. Note, I had thoughts of adding some cross-origin checks as well, so I wrote the test to be a bit extensible. I decided to punt on that for now, though. This initial start on the test at least verifies we inherit properly for the typical blob case.
Attachment #8944481 -
Flags: review?(bugmail)
Updated•6 years ago
|
Attachment #8944062 -
Flags: review?(bugmail) → review+
Comment 6•6 years ago
|
||
Comment on attachment 8944481 [details] [diff] [review] P2 Add blob-url-inherits-controller.https.html wpt test. r=asuth Review of attachment 8944481 [details] [diff] [review]: ----------------------------------------------------------------- Great test. ::: testing/web-platform/tests/service-workers/service-worker/resources/blob-url-inherit-controller-frame.html @@ +2,5 @@ > +<html> > +<script> > +function getBlobText(opts) { > + if (opts.child === 'iframe') { > + return '<script>' + No action needed, but don't forget template literals (`blah`) that support newlines exist. (And if you want the byproduct to be newline free, a tagged literal function like "norm = x => x.join('').replace(/\n */g, ' ')" can accomplish that for norm`stuff\n blah`. Although obviously if you want the interpolation to still work, something more is needed.)
Attachment #8944481 -
Flags: review?(bugmail) → review+
Assignee | ||
Comment 7•6 years ago
|
||
Addressed review feedback about strings. Also, I added checks for data URLs. We don't inherit in that case because they get an opaque origin.
Attachment #8944481 -
Attachment is obsolete: true
Attachment #8944786 -
Flags: review+
Pushed by bkelly@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/28e035f10d0a P1 Allow the controller to be inheritted for other local URLs like blob: and data:. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/4fe6b13848c8 P2 Add local-url-inherit-controller.https.html wpt test. r=asuth
Comment 9•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/28e035f10d0a https://hg.mozilla.org/mozilla-central/rev/4fe6b13848c8
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in
before you can comment on or make changes to this bug.
Description
•