691763 - Fennec Layout is broken on initial startup after landing bug 688505

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Oleg Romashin (:romaxa)]

On first startup fennec layout looks broken,  side/tool bars overlapping each other.
rotation/resize is needed in order to restore normal behavior
after bisecting I've found 
changeset:   77851:3e60b13624dc
user:        Wes Johnston <[e-mail]>
date:        Thu Sep 22 17:04:15 2011 -0700
summary:     Bug 688505 - Use keyhole shape for back button on Honeycomb. r=mfinkle

Comment 1

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

Is this the same layout issue as in bug 689928 ?

Comment 2

[Oleg Romashin (:romaxa)]

Found that removing svg:svg entry from browser.xul fixes the problem...
is it android only?

Comment 3

[Oleg Romashin (:romaxa)]

No it is different, I see right side bar about in the middle of screen, or even sometimes right after left sidebar

Comment 4

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

Got a screenshot of the problem? What device?

Comment 5

[Oleg Romashin (:romaxa)]

Attachment: Possible patch

I've changed svg:svg -> just svg, and it started working fine

Comment 6

[Aaron Train [:aaronmt]]

I don't see the aforementioned abnormalities on 10/04 (SII) and 10/04 (Tab 10.1).

Comment 7

[Oleg Romashin (:romaxa)]

(In reply to Mark Finkle (:mfinkle) from comment #1)
> Is this the same layout issue as in bug 689928 ?

it could be in case if initial startup on android having more main window resizes than in Maemo...

Comment 8

[Oleg Romashin (:romaxa)]

Attachment: Photo of broken layout1

Comment 9

[Oleg Romashin (:romaxa)]

Attachment: Photo of broken layout 2

Comment 10

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

Comment on attachment 564559 [details] [diff] [review]
Possible patch

This probably kills the SVG altogether

Comment 11

[Oleg Romashin (:romaxa)]

I've noticed one more thing, 
if I start fennec without any url command, then broken layout only visible in the beginning and then replaced with FF home page..
but if I start fennec with some url in command line like,
./fennec http://google.com - then layout is broken and stays broken until resize  happen.

Comment 12

[Oleg Romashin (:romaxa)]

> This probably kills the SVG altogether
why that kills SVG? I see svg still working even with that patch, at least element created and no parsing errors visible

Comment 13

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

Comment on attachment 564559 [details] [diff] [review]
Possible patch

OK. Pushing to Wes so he can test that this does not break what he did in the original bug.

Comment 14

[Wesley Johnston (:wesj)]

Comment on attachment 564559 [details] [diff] [review]
Possible patch

Review of attachment 564559 [details] [diff] [review]:
-----------------------------------------------------------------

This is invalid markup and breaks the mask in our Honeycomb theme (i.e. try highlighting the forward button).

Comment 15

[Oleg Romashin (:romaxa)]

Checked more deeply resizeHandler calls, and found that before that fix we have 5 nsWindow::OnResizeEvent events, and that triggering fennec resizeHandler... 
after that fix we have only 3 nsWindow::OnResizeEvent calls and fennec resizeHandler never triggered

Comment 16

[Oleg Romashin (:romaxa)]

Attachment: resize log before and after

Comment 17

[Oleg Romashin (:romaxa)]

Attachment: Call first resize by timeout in case if native View already resized

ok, we do call DispatchResizeEvent in nsWindow after resizeHandler registered, but it does not reach PresShell::ResizeReflow, because size is the same and it is blocked in nsViewManager::DoSetWindowDimensions.
so currently we don't see this problem on android because native widget is not sized properly before fennec register resizeHandler.

Comment 18

[Oleg Romashin (:romaxa)]

Attachment: Call first resize by timeout in case if native View already resized

Make it MAEMO only

Comment 19

[Oleg Romashin (:romaxa)]

Attachment: Don't specify xulwindow size if fullscreen size specified

Ok, I've found real problem which is causing wrong resize event 480,480 during startup.
even if widget sized properly after initial startup, we still get event before startup with bad size, which is coming from nsXULWindow::LoadSizeFromXUL, 
that code calculating spec size based on xul window params, and ignoring fullscreen definition...

Here is the simple fix which does not specify width, height for maemo, because fullscreen size mode already specified.

Comment 20

[Matt Brubeck (:mbrubeck)]

Comment on attachment 565337 [details] [diff] [review]
Don't specify xulwindow size if fullscreen size specified

I wonder whether we should get rid of these on Android too.

Comment 21

[Matt Brubeck (:mbrubeck)]

Comment on attachment 565337 [details] [diff] [review]
Don't specify xulwindow size if fullscreen size specified

Requesting mozilla-approval-aurora for Firefox 9, at romaxa's request.  This fixes a Maemo-only regression that breaks the community-maintained Maemo port of Fennec.  The fix is also Maemo-only; it does not change a single line of code for other platforms.  It just eliminates (on Maemo) hard-coded sizes in our XUL.

Comment 22

[Ed Morley [:emorley]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/22f7450fdc4b

Comment 23

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/327b44552e6f

Comment 24

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/22f7450fdc4b

Comment 25

[Camelia Urian]

Build ID: Mozilla/5.0 (Maemo; Linux armv7l; rv:9.0a2) Gecko/20111019 Firefox/9.0a2 Fennec/9.0a2
Device: Nokia N900 - Maemo 5

Issue was verified on above environment on landscape and portrait, with FF start page and also starting fennec with some url in command line like: "fennec http://google.com" - Fennc layout was not broken.