310793 - loaded page breaks after using (xpi + browser.link.open_external:1) - context menus, reload, forms all broken.

Status: NEW

(Core :: DOM: Navigation, defect)

Description

[Przemyslaw Bialik]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20050930 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20050930 Firefox/1.6a1

Bug 300114 (fixed by bug 271567) is not anymore but after accepting/rejecting
xpinstall dialog, page in active tab is not fully usable (probably caused by Bug
298255 and IIRC this wasn't the case in FF 1.0.4).
Details below.

Reproducible: Always

Steps to Reproduce:
1. Load any page, e.g. http://www.extensionsmirror.nl/ ;)
2. Double click on local xpi (accept/reject xpinstall dialog)
with setting (browser.link.open_external:1) = Tools -> Options... -> Tabs -
'Open links from
other applications in:' - 'the most recent tab/window'
3. Check if loaded page if fully usable as it was before

Actual Results:  
The page is not fully usable, you can't: use context menu, refresh the page,
type in edit boxes etc.

Expected Results:  
a) The page is fully usable (preferred)
b) blank currently loaded page before showing xpinstall dialog so user could go
back to fully working page (do not blank if the current page is about:blank)

Comment 1

[Asa Dotzler [:asa]]

Dan, can you dig into this?

Comment 2

[Daniel Veditz [:dveditz]]

This is not unique to xpinstall, this happens for any type not handled by
layout. Mostly those types are externally handled, so if you double-clicked in
the OS you'd never go through Firefox in the first place (on the other hand,
xpinstall doesn't go to Firefox by default either). But if you force the issue
by passing the file on the commandline, then you get the same result.

Example: Assume you have pdf's set to open in the external reader (not the
plugin). firefox "file:///c:/path/myfile.pdf" will switch the current tab title
to "Untitled", then pass the PDF to the external handler. The existing content
will be broken in the same way as this bug describes for xpinstall.

Confirming bug. I don't see it as a blocker:
- replace current content is not the default behavior
- it's not a new problem (though masked for .xpi until recently)
- only a minority have local .xpi to click on
- by default .xpi doesn't open in Firefox anyway

The most annoying thing is that the original page was not entered into session
history when we started loading the new content, so you can't "go back" to it.
But apparently we started enough on the new page that you can't "reload" it either.

Comment 3

[Daniel Veditz [:dveditz]]

Oh, and the 1.0x branch doesn't have this problem, so it's something that
regressed at some point on the trunk.

Comment 4

[Przemyslaw Bialik]

(In reply to comment #2)
> - replace current content is not the default behavior
yeah now it isn't but wouldn't the majority of users reuse their 1.0.x profiles ?

> - by default .xpi doesn't open in Firefox anyway
hmm maybe I did it long time ago myself but like firefox associates itself with
.html etc. extensions it should do the same with .xpi (especially that this file
extension is rather only used by mozilla browsers) ?
Anyway I can agree that this bug affects a minority of firefox users.

(In reply to comment #3)
> Oh, and the 1.0x branch doesn't have this problem, so it's something that
> regressed at some point on the trunk.
huh ? I've noticed this behaviour when reporting Bug 300114 but I didn't said
anything about it because I had thought that patch to this bug would fix also
'page breaking'.
Note that on 1.0.5+ all what I've said in this bug applies beside that you can
reload the page but you can't scroll the page.
Nightly trunk behaviour matches nightly branch behaviour.

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.12) Gecko/20050915
Firefox/1.0.7

Comment 5

[Boris Zbarsky [:bzbarsky]]

So the issue here is the CreateAboutBlankContentViewer() call that we make in the aLoadType == LOAD_NORMAL_EXTERNAL call, right?

So what behavior do we want here?  Given how the JS protocol handler works, we have to clear before AsyncOpen.  But we won't know where we're targeting (or whether we can handle) until after AsyncOpen...

Perhaps we should simply remove support for the "current window" value of this pref?  I see no way to implement it safely and without breaking things like this.

Comment 6

[[e-mail]]

The behaviour I'd want is for -url to run the URL through the doc loader and only if it generated HTML¹ content would it look for a docshell to load in.

¹Or other supported MIME types, of course.

Comment 7

[Boris Zbarsky [:bzbarsky]]

> run the URL through the doc loader

That requires having a docshell at the moment (if nothing else, "javascript:" URIs need a script context to run against).