Closed Bug 1214666 Opened 9 years ago Closed 9 years ago

AboutRedirector does not work for resources loaded by about:loop* pages with relative URLs

Categories

(Hello (Loop) :: Client, defect, P1)

defect

Tracking

(e10sm8+)

RESOLVED WORKSFORME
Iteration:
44.2 - Oct 19
Tracking Status
e10s m8+ ---

People

(Reporter: mikedeboer, Assigned: mrbkap)

References

Details

(Whiteboard: [e10s][44])

Attachments

(1 file)

NOTE: summary and product fields of this bug may change in the future is insight into this issue improves.

When opening the 'about:looppanel' page in a remote browser, each external stylesheet and script document that should be loaded fail whilst showing the following message in the terminal:

[Child 91150] WARNING: Failed to create a URI: file /Users/mikedeboer/Projects/fx-team/parser/html/nsHtml5TreeOpExecutor.cpp, line 897

After some investigation and printf-debugging, it appeared that for nsDocument instances for these about: pages (and I'm assuming others, too) don't have their `mDocumentBaseURI` property set to the chrome:// URL that the AboutRedirector should translate to.
In other words: nsDocument::GetDocBaseURI() falsely returns `mDocumentURI` instead of `mDocumentBaseURI` when about: pages are running in the content process, whereas currently it's the other way around.
Flags: qe-verify-
Flags: firefox-backlog+
Blcoker for Hello working properly with e10s
tracking-e10s: --- → ?
(In reply to Mark Banner (:standard8) from comment #1)
> Blcoker for Hello working properly with e10s

and I should have added, this is also going to be blocking some other Loop work, as it stops us having tab sharing working, which is going to be critical for the part we're working on next (bug 1209713).
Hey mikedeboer, did you ever get my test-case working? That'd probably help here to more clearly illustrate the problem to the folks who'd likely be able to fix it.
Flags: needinfo?(mdeboer)
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #3)
> Hey mikedeboer, did you ever get my test-case working? That'd probably help
> here to more clearly illustrate the problem to the folks who'd likely be
> able to fix it.

Working on it!
Flags: needinfo?(mdeboer)
Rank: 8
Assignee: nobody → mrbkap
Figured out that referencing a script or stylesheet relative to the location of the document doesn't work.
The cause may still be a `SetDocumentBaseURI()` being omitted.
Summary: AboutRedirector does not work for resources loaded by about:loop* pages → AboutRedirector does not work for resources loaded by about:loop* pages with relative URLs
I started looking into this today and there are a couple of things that I don't understand:

1) I'm debugging this entirely in the parent because as far as I can tell, about:looppanel is happy to load in the parent if I type it into the URL bar (note that I still don't see any code setting a base URI in the parent). Comment 0 refers to loading in the child process, so I'm wondering if there are steps that I'm missing here.

2) Even if I change panel.html to use absolute chrome:// URLs things don't work, but that could be because it's not intended to be loaded as a top-level page.

So, how is about:looppanel supposed to be used? What are the steps to reproduce that I should be following here? Also, what is the mysterious testcase talked about in comment 3?
Flags: needinfo?(mdeboer)
Like I said on IRC earlier, mconley gave me the scaffolding for a comprehensive (regression) test, but I didn't manage to easily reproduce the problem; or: I could not get the messages printed as I did with printing them directly with printf in various places from C-code.

This patch is what you can apply to make the panel run in the content process. I put an alert() inside one of the scripts which _should_ fire when everything working dandy. But since it's not, you won't see the alert.

When you comment out `attrs.remote = true;` and `attrs.panelFrameType = aType;` in PanelFrame.jsm after you applied the patch, the panel will load in the main process again and BLAM! the alert fires.
Flags: needinfo?(mdeboer)
I mentioned this in IRC, but putting it here for posterity:

One possible reason why the test case might not be reproducing the problem correctly is that the test creates an nsIAboutModule through the addon shims.

The about:loop stuff is registered in the browser's AboutRedirector, and that won't use the shims, naturally.
Plus the external resources (JS & CSS files) are specific in the browsers' jar.mn file, so they're accessible through the same base URL. Might be a hint?
I spoke to sicking about this and things are working as intended here. The trick is that about: redirects are not implemented at all the same as HTTP redirects. They're loaded from about: URIs with an underlying channel that "secretly" loads from a different URI.

This is why every about: page in the tree uses absolute paths. You should be able to work around this by using <base href="chrome://browser/content"> in your panel.html. I tested that without the patch from comment 7 and things seemed to work correctly, but with the patch, I get:

  JavaScript error: chrome://browser/content/loop/libs/l10n.js, line 106: TypeError: gL10nDetails is undefined

which means that navigator.mozLoop isn't being injected properly (which seems to break things really badly, I get some *really* odd results). I haven't looked into why that is, but I can if you need me to. It does mean that we're loading the JS files and running them.
Flags: needinfo?(mdeboer)
(In reply to Blake Kaplan (:mrbkap) (please use needinfo!) from comment #10)
> I spoke to sicking about this and things are working as intended here. The
> trick is that about: redirects are not implemented at all the same as HTTP
> redirects. They're loaded from about: URIs with an underlying channel that
> "secretly" loads from a different URI.

This I knew, but the odd thing is that relative paths are working fine for us at the moment, but it stops working when running in the content process.

> This is why every about: page in the tree uses absolute paths. You should be
> able to work around this by using <base href="chrome://browser/content"> in
> your panel.html. I tested that without the patch from comment 7 and things
> seemed to work correctly, but with the patch, I get:

I did the same and blimey did it work! Thanks for the tip ;-)

>   JavaScript error: chrome://browser/content/loop/libs/l10n.js, line 106:
> TypeError: gL10nDetails is undefined

Never mind these error, they're there because of a different reason.
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: needinfo?(mdeboer)
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: