1460939 - Reload of wyciwyg toplevel page coming from file:// is broken in e10s

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P3)

Description

[Boris Zbarsky [:bzbarsky]]

Attachment: baz.html

See attached testcase.  On tip, it reloads to a blank page, with this error in the console:

JavaScript error: chrome://global/content/browser-child.js, line 352: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIWebNavigation.loadURIWithOptions]

It used to reload to a page showing "Test".  This broke somewhere in the 55 cycle, and if I grab the 2017-02-01 nightly it's broken in e10s mode but works in non-e10s mode.  So it's possible this never worked in e10s mode...

Mike, do you know who would be the right person to look at this?

Olli, do you think we should just give up on wyciwyg completely?

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Hey bz,

I think I'm misunderstanding the STR here. Here's what I'm doing:

1) Browse to https://bugzilla.mozilla.org/attachment.cgi?id=8975099
2) Click on the <div>
3) Reload the page

On today's Nightly on both macOS and Windows, as well as the May 11th Nightly (when you filed the bug), I still see "Test" after reloading. Are my STR off?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Ah, looks like it works for http but not file://; I should have tested both.  Try downloading the testcase and running from a file:// URI.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

If it works for http, removing support sounds a bit risky.
But, it might be worth to do it anyhow. Just load the page from scratch and let it run its document.write stuff again.
Would be good to have a pref for the removal first, and enable perhaps Nightly only.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

I did some bisection. Unfortunately, there are some busted or missing taskcluster builds around that time, so the smallest range I could get was:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bad312aefb42982f492ad2cf36f4c6c3d698f4f7&tochange=f8f4eaac1701107f794b48891bcca2c95d39d503

but I'm 98% certain this was caused by bug 1147911.

I'm also not sure this is worth fixing, fwiw. wyciwyg support for file:/// documents that do document.write-y things sounds pretty niche.

Comment 5

[:Gijs (he/him)]

Wonder if this is related to bug 1402418 as well...

Comment 6

[Boris Zbarsky [:bzbarsky]]

Oh, hmm.  Are we not putting the wyciwyg://file:// load in the same process as file:// ?

wyciwyg support for file:// might be less niche for the web, given WebKit's lack of support for it altogether....

Comment 7

[Bob Owen (:bobowen)]

We don't have a special case for wyciwyg in E10SUtils.jsm getRemoteTypeForURIObject and I don't think we ever have, so they default to web/http remote type.

I'm guessing that they aren't nsINestedURIs, because I think if they were it would handle this correctly.

Comment 8

[Boris Zbarsky [:bzbarsky]]

> I'm guessing that they aren't nsINestedURIs

That's correct.  People who need to deal with them have to manually parse some bits out of them.  See nsContentUtils::RemoveWyciwygScheme.

Comment 9

[Andrew McCreight [:mccr8]]

wyciwyg is going away so I think this will become a non-issue.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Fixed by bug 1489308.