519099 - Location bar stops working and does not allow any input. Restarts does not help

Status: VERIFIED FIXED

(Firefox :: Session Restore, defect, P2)

Description

[José Jeria]

Build ID:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20090926 Minefield/3.7a1pre

STEPS TO REPRODUCE
1. Make sure you have "Show my windows and tabs from last time" enabled
2. Go to http://www.chromeexperiments.com/detail/monster/
3. Click on "Launch Experiment"
4. Close the main window, leaving the popup as the only window open
5. Exit Firefox
6. Restart Firefox
7. It will not be possible to type anything in the Location bar

ADDITIONAL INFO
You wont be able to type anything into the Location bar now, restarting Firefox wont help. Creating a new tab will not help either.

Only way to get out of this "locked" mode is to open a new window

Tested on Windows and could not reproduce, MAC OS X only?

Comment 1

[David Dahl :ddahl]

Cannot reproduce on Linux build: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20090928 Minefield/3.7a1pre

Comment 2

[Dietrich Ayala (:dietrich)]

can someone try to reproduce this on a mac?

Comment 3

[Drew Willcoxon :adw]

I can reproduce on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20090928 Minefield/3.7a1pre.

Comment 4

[Dietrich Ayala (:dietrich)]

denying blocking until there's independent verification.

reporter: can you please try to reproduce this in safe mode?

http://support.mozilla.com/en-US/kb/Safe+Mode

Comment 5

[Shawn Wilsher :sdwilsh]

(In reply to comment #4)
> denying blocking until there's independent verification.
comment 3?

Comment 6

[Drew Willcoxon :adw]

The location bar's readonly and isempty attributes are "true", even after restart, but I don't see where these atts are persisted.

Comment 7

[Tracy Walker [:tracy]]

I can not reproduce this on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a1pre) Gecko/20090928 Minefield/3.7a1pre

However, I'm not sure what the reporter refers to in step 1; have "Show my windows and tabs from last time" enabled. There is a shutdown dialog similar to that when more than one window or tab is open.  Step 4 ensures that dialog won't appear.  I can not reproduce this with multiple tabs/windows open nor with only the popup only.

Comment 8

[Drew Willcoxon :adw]

(In reply to comment #7)
> However, I'm not sure what the reporter refers to in step 1; have "Show my
> windows and tabs from last time" enabled.

Minefield > Preferences > General tab > When Minefield starts: Show my windows and tabs from last time

Comment 9

[Tracy Walker [:tracy]]

ah, yes, I forgot about that one.  I can reproduce as reported with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a1pre) Gecko/20090928 Minefield/3.7a1pre

Comment 10

[Dietrich Ayala (:dietrich)]

blocking. can we get a regression range?

Comment 11

[Drew Willcoxon :adw]

Looking at the resulting sessionstore.js, the isPopup property of the page is true, which sets the location bar to readonly on restore:
http://mxr.mozilla.org/mozilla-central/source/browser/components/sessionstore/src/nsSessionStore.js#2366

Comment 12

[Drew Willcoxon :adw]

The location bar of the popup that opens after step 3 in comment 0 is readonly, so it makes sense that when the window is restored it remains readonly, no?

Comment 13

[Tracy Walker [:tracy]]

Can not repro on XP. With the difference in they way Mac and XP do Window?Menu handling. On XP with the popup being menuless, restarting minefield opens the popup and the last window closed (one with main menu).   That window's location bar is functional.

Comment 14

[juan becerra [:juanb]]

On mac the behavior changed between 7/23 and 7/28 on mozilla central. I'll narrow it down a little further.

Comment 15

[juan becerra [:juanb]]

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0322169bc93e&tochange=737a77143f49

Comment 16

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

So I actually think this is expected behavior right now... Starting with bug 481090 we special cased OSX to not be too tricky. Things did get a bit hairier after I landed bug 394759.

This would be the guilty changeset: http://hg.mozilla.org/mozilla-central/rev/9b8f46d71e4e - which means it should also be causing problems currently with Firefox 3.5 :(

We're explicitly disabling the location bar on restore for popups. The location bar is disabled before shutdown too, so it shouldn't be _that_ surprising.

Moving to sessionstore where this fits better.

Comment 17

[Tracy Walker [:tracy]]

(In reply to comment #16)

> 
> This would be the guilty changeset:
> http://hg.mozilla.org/mozilla-central/rev/9b8f46d71e4e - which means it should
> also be causing problems currently with Firefox 3.5 :(
> 

Yes, it is on 3.5

Comment 18

[Tracy Walker [:tracy]]

(In reply to comment #16)
 
> We're explicitly disabling the location bar on restore for popups. The location
> bar is disabled before shutdown too, so it shouldn't be _that_ surprising.
> 
It's unexpected because on restart it is not a popup window that is opened. What is opened is a browser window which happens to have broken location bar.

Comment 19

[Mike Beltzner [:beltzner, not reading bugmail]]

This reminds me of a bug around Firefox 3.5 time, although in that case there was a workaround - see bug 495495. Re-marking as P2 blocker (flag was removed in error)

Comment 20

[Shawn Wilsher :sdwilsh]

Is this a session restore issue, or is it an issue with places code (and the location bar)?

Comment 21

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Ok, so what's the expected behavior then? If we're restoring a popup window, do we just turn that into a normal window? Or do we essentially backout bug 481090?

Also, I assume then that if we make the window normal, we ignore the other window features that we try to restore ("menubar", "toolbar", "locationbar", "personalbar", "statusbar", "scrollbars")? (Ignore the fact that scrollbars doesn't actually behave correctly, see bug 485955)

Comment 22

[Drew Willcoxon :adw]

(In reply to comment #21)
> Ok, so what's the expected behavior then?

In comment 13 Tracy says that on Windows a "real" window is always restored; the popup + the final real window are both restored, even if the popup was actually the final window.

Comment 23

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to comment #22)
> In comment 13 Tracy says that on Windows a "real" window is always restored;
> the popup + the final real window are both restored, even if the popup was
> actually the final window.

Indeed. But we specifically disabled that on OSX in bug 
481090.

Comment 24

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

limi: "the tl;dr summary: there are some fundamental issues with how we do this that should be fixed in the next release — in the meantime: restore the window, but un-gimp it when it's restored, so if people start using it as a normal window, it behaves like one"

Comment 25

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

So I was a little wrong in my diagnosis before. It's a combination of things that didn't consider this edge case.

On startup we do ignore (ie. don't hide) all features for the first window that were hidden on shutdown (eg. location bar). This was good enough. On quit we always made sure the first window wasn't a popup. Then came bug 481090 which changed that on OS X. This was still ok though because even if the window had .isPopup, we were still enabling all features. Then bug 495495 ensured that if .isPopup we explicitly disabled the location bar again.

So there's a 1 line fix to remove isPopup at startup. However this doesn't affect Private Browsing mode. So if there was only a popup with location bar disabled going into PB, the popup will be restored with the location bar disabled. I think that's ok and probably closer to what the user expects. This is the better fix in my opinion.

I could make the fix more complicated and clear hidden/isPopup at shutdown for OSX or in _getCurrentState (but that's a change that would impact consumers of the API). I don't really want to do either of these.

Comment 26

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.1

Takes approach 1 from previous comment.

Comment 27

[Simon Bünzli]

Comment on attachment 406468 [details] [diff] [review]
Patch v0.1

r+=me with thanks.

Comment 28

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Pushed http://hg.mozilla.org/mozilla-central/rev/a6273493eab4

and http://hg.mozilla.org/releases/mozilla-1.9.2/rev/467098ec0cb9

Asking for blocking on 1.9.1 since this is a regression on that branch (starting with 1.9.1.2) - regression landed here: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/8cbe12f3a208

Comment 29

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

We'll need a litmus test for this since it's only in the startup path.

Comment 30

[Daniel Veditz [:dveditz]]

If this is the patch you want to land please request approval1.9.1.5 on it. Otherwise create the back-port and request approval on that.

Comment 31

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Comment on attachment 406468 [details] [diff] [review]
Patch v0.1

Patch applies cleanly to 1.9.1 (and more importantly, works). No backport necessary.

Comment 32

[Daniel Veditz [:dveditz]]

Comment on attachment 406468 [details] [diff] [review]
Patch v0.1

Approved for 1.9.1.5, a=dveditz for release-drivers

Comment 33

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/1a90838e2043

Comment 34

[José Jeria]

Will this also land on the Firefox 3.6 branch?

Comment 35

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to comment #34)
> Will this also land on the Firefox 3.6 branch?

It has. http://hg.mozilla.org/releases/mozilla-1.9.2/rev/467098ec0cb9 (mentioned in comment #28). It landed after the beta, so if you're still seeing the problem, that would be why. It should be fixed in nightlies though.

Comment 36

[Tracy Walker [:tracy]]

verified with: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a1pre) Gecko/20091103 Minefield/3.7a1pre

Comment 37

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed on 1.9.2 and 1.9.1 with

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b3pre) Gecko/20091109 Namoroka/3.6b3pre ID:20091109035432

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.6pre) Gecko/20091108 Shiretoko/3.5.6pre ID:20091108030959

Comment 38

[u279076]

3 test cases added:
https://litmus.mozilla.org/show_test.cgi?id=11513
https://litmus.mozilla.org/show_test.cgi?id=11514
https://litmus.mozilla.org/show_test.cgi?id=11515

Marking in-litmus+