602434 - urlbar disappears when opening the edit bookmarks dialog and the softkb popping up

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aakash Desai [:aakashd]]

Build Id:
Mozilla/5.0 (Android; Linux armv7l; rv:2.0b7pre) Gecko/20101006 Firefox/4.0b7pre Fennec/4.0b1

Steps to Reproduce:
1. Swipe left to open the controls sidebar
2. Tap on the bookmarks star button
3. Tap on the "Edit" button 

Actual Results:
The edit bookmarks dialog and soft keyboard pop up, but the urlbar disappears.

Expected Results:
All three pop up.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Wes - can you investigate this and see what's happening.

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

On my side I suspect that the window is resize, which involve to refresh the zoom which itself call hideSidebars & co.

Comment 3

[Wesley Johnston (:wesj)]

Attachment: Quickfix

I think you're right. The forceChromeReflow combined with the resize event seems to cause problems... I can look into this harder, but if we really want to make sure the urlbar stay visible for this one popup, it seems reasonable to me to just lock it.

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #3)
> Created attachment 481658 [details] [diff] [review]
> Quickfix
> 
> I think you're right. The forceChromeReflow combined with the resize event
> seems to cause problems... I can look into this harder, but if we really want
> to make sure the urlbar stay visible for this one popup, it seems reasonable to
> me to just lock it.

We could go with this solution, but I really would like to remove to remove the real bug. Could you have a look to http://mxr.mozilla.org/mobile-browser/source/chrome/content/AnimatedZoom.js#60 and see if removing those lines has any bad side effects? They are causing a whole bunch of others unexpected behavior I don't like, as for example hiding the titlebar when the orientation is changed, etc..

If you found some bad behavior I think we should go with your fix and open an other bug to fix AnimatedZoom

Comment 5

[Wesley Johnston (:wesj)]

I don't see any bad effects from playing with those values in AnimatedZoom, but they don't solve this problem either.

Digging a little harder, I think the real issue is:

http://mxr.mozilla.org/mobile-browser/source/chrome/content/browser.js#268

which fires on the resize, hides the sidebars, and in the process can hide the urlbar. Tomorrow I'll play with moving it into plain old startup and see what happens. You can reproduce this fairly well on desktop by bringing up the bookmarks popup and then resizing the window.

Comment 6

[Wesley Johnston (:wesj)]

Fixing the resize stuff is going to be tough I think. When you go from portrait->landscape the sidebars are shown, so we force them to hide regardless of their original state. We should at least keep the upper left corners of each scrollbox fixed during resize, but I think a reflow is happening which screws things up. IMO, we should just file a follow up to remove those hacks, but take this patch to fix this case.

I've played with those lines in AnimatedZoom. There are no glaring bugs without them, and all of our browser-chrome tests pass. I think we can delete them, but I feel like its completely unrelated, and I don't like mixing unrelated patches.

Tell me if you disagree or want something else here.

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 481658 [details] [diff] [review]
Quickfix

Let's fix it this way for now. We can kick the zoom/resize bug elsewhere

Comment 8

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

http://hg.mozilla.org/mobile-browser/rev/d46fdbdbe45f

Comment 9

[Wesley Johnston (:wesj)]

I filed Bug 604425 for a follup on the resize stuff.

Comment 10

[Aakash Desai [:aakashd]]

verified FIXED on builds (vibrant and nexus one):
Mozilla/5.0 (Android; Linux armv71; rv:2.0b8pre) Gecko/20101015 Namoroka/4.0b8pre Fennec/4.0b2pre