Closed Bug 1317036 Opened 8 years ago Closed 6 years ago

Source showed in view-source doesn't respect service worker

Categories

(Core :: DOM: Service Workers, defect, P2)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: xidorn, Unassigned)

References

(Blocks 1 open bug)

Details

Steps to reproduce:
1. open https://www.upsuper.org/
2. click "View source" on the footer of the page
3. in the new page, click "View Page Source" in the context menu

Expected result:
The view-source page should show the source of the page being viewed (which has been rewritten by Service Worker, and its actual address is https://www.upsuper.org/index_html-src.html)

Actual result:
The view-source page shows the source of the original URL, not respecting the Service Worker.


(I'm not sure whether this is actually a bug, because for this case I do prefer Firefox's behavior that the source page has identical content to the page itself :)
Hmm, :bkelly, any thoughts on what you'd expect with view source for pages fetched by service workers?

I guess showing the document as fetched by the SW would seem like right thing to do to match non-SW content.
Flags: needinfo?(bkelly)
Priority: -- → P2
I believe we need to use NS_GetInnermostURI() here and perhaps in some other places in SW code:

https://dxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#14576
Flags: needinfo?(bkelly)
I tried just adding it there, but it was not adequate.  We probably need to audit all uses of nsIURI within ServiceWorkerManager to use the inner most URI.
Component: View Source → DOM: Service Workers
Product: Toolkit → Core
It seems we are making view-source: windows a different origin in bug 1430257.  Its unclear to me if we should be intercepting the underlying source load or not any more.  We should definitely not make the window controlled.
See Also: → 1430257
Xidorn, is this working for you now?  Looking at comment 0 I think it is, but I can't remember exactly what the failure case looks like any more.  It seems this might have started working with bug 1231211.
Flags: needinfo?(xidorn+moz)
I think I'm still seeing the old result that view-source:https://www.upsuper.org/index.html is showing the source of index.html rather than index_html-src.html which is the page returned by the service worker.
Flags: needinfo?(xidorn+moz)
So I do see the service worker throwing an error:

TypeError: Request mode 'no-cors' was used, but request cache mode 'only-if-cached' can only be used with request mode 'same-origin'.

This happens on the part where you try to mint a new Request with a no-cors mode.  Our view-source request sets "only-if-cached" which is only compatible with "same-origin" mode.  I wonder if this exception is why you are not getting the request intercepted.

I think we have relaxed some of the requirements for new Request() now.  Maybe you can change this code to just do:

  let r = new Request(newURL, evt.request);

Or replace "no-cors" with "same-origin" in your code.
Flags: needinfo?(xidorn+moz)
Actually, I think I found the bug.  The view-source code does *not* set LOAD_ONLY_FROM_CACHE.  We're mis-interpreting the load flags in service workers.
Flags: needinfo?(xidorn+moz)
Xidorn, can you try with the latest nightly 60?  I believe bug 1437080 probably fixed this.
Flags: needinfo?(xidorn+moz)
Yeah, it works now. Although there is a separate regression somehow now serving the source code for the svg images...
Flags: needinfo?(xidorn+moz)
Marking fixed for now then since you wrote another bug for the svg fragment issue.  Thanks.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.