Last Comment Bug 691763 - Fennec Layout is broken on initial startup after landing bug 688505
: Fennec Layout is broken on initial startup after landing bug 688505
Status: VERIFIED FIXED
[fixed in aurora]
: regression
Product: Fennec Graveyard
Classification: Graveyard
Component: General (show other bugs)
: Trunk
: ARM Maemo
: -- normal (vote)
: Firefox 9
Assigned To: Oleg Romashin (:romaxa)
:
Mentors:
Depends on:
Blocks: 688505
  Show dependency treegraph
 
Reported: 2011-10-04 08:13 PDT by Oleg Romashin (:romaxa)
Modified: 2011-10-21 02:27 PDT (History)
6 users (show)
emorley: in‑testsuite-
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Possible patch (904 bytes, patch)
2011-10-04 08:32 PDT, Oleg Romashin (:romaxa)
wjohnston2000: review-
Details | Diff | Review
Photo of broken layout1 (41.84 KB, image/jpeg)
2011-10-04 09:02 PDT, Oleg Romashin (:romaxa)
no flags Details
Photo of broken layout 2 (41.71 KB, image/jpeg)
2011-10-04 09:03 PDT, Oleg Romashin (:romaxa)
no flags Details
resize log before and after (3.22 KB, text/plain)
2011-10-04 16:21 PDT, Oleg Romashin (:romaxa)
no flags Details
Call first resize by timeout in case if native View already resized (2.44 KB, patch)
2011-10-04 18:24 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Review
Call first resize by timeout in case if native View already resized (2.47 KB, patch)
2011-10-05 22:00 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Review
Don't specify xulwindow size if fullscreen size specified (1.20 KB, patch)
2011-10-06 13:38 PDT, Oleg Romashin (:romaxa)
mbrubeck: review+
asa: approval‑mozilla‑aurora+
Details | Diff | Review

Description Oleg Romashin (:romaxa) 2011-10-04 08:13:24 PDT
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 <wjohnston@mozilla.com>
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?) 2011-10-04 08:14:50 PDT
Is this the same layout issue as in bug 689928 ?
Comment 2 Oleg Romashin (:romaxa) 2011-10-04 08:26:00 PDT
Found that removing svg:svg entry from browser.xul fixes the problem...
is it android only?
Comment 3 Oleg Romashin (:romaxa) 2011-10-04 08:27:35 PDT
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?) 2011-10-04 08:29:56 PDT
Got a screenshot of the problem? What device?
Comment 5 Oleg Romashin (:romaxa) 2011-10-04 08:32:47 PDT
Created attachment 564559 [details] [diff] [review]
Possible patch

I've changed svg:svg -> just svg, and it started working fine
Comment 6 Aaron Train [:aaronmt] 2011-10-04 08:34:44 PDT
I don't see the aforementioned abnormalities on 10/04 (SII) and 10/04 (Tab 10.1).
Comment 7 Oleg Romashin (:romaxa) 2011-10-04 08:36:27 PDT
(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) 2011-10-04 09:02:11 PDT
Created attachment 564570 [details]
Photo of broken layout1
Comment 9 Oleg Romashin (:romaxa) 2011-10-04 09:03:06 PDT
Created attachment 564571 [details]
Photo of broken layout 2
Comment 10 Mark Finkle (:mfinkle) (use needinfo?) 2011-10-04 09:05:30 PDT
Comment on attachment 564559 [details] [diff] [review]
Possible patch

This probably kills the SVG altogether
Comment 11 Oleg Romashin (:romaxa) 2011-10-04 09:05:54 PDT
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) 2011-10-04 09:07:53 PDT
> 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?) 2011-10-04 09:12:38 PDT
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) 2011-10-04 09:40:51 PDT
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) 2011-10-04 16:18:30 PDT
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) 2011-10-04 16:21:56 PDT
Created attachment 564699 [details]
resize log before and after
Comment 17 Oleg Romashin (:romaxa) 2011-10-04 18:24:58 PDT
Created attachment 564733 [details] [diff] [review]
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) 2011-10-05 22:00:28 PDT
Created attachment 565132 [details] [diff] [review]
Call first resize by timeout in case if native View already resized

Make it MAEMO only
Comment 19 Oleg Romashin (:romaxa) 2011-10-06 13:38:45 PDT
Created attachment 565337 [details] [diff] [review]
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) 2011-10-06 13:39:46 PDT
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) 2011-10-06 14:28:25 PDT
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 23 Matt Brubeck (:mbrubeck) 2011-10-07 10:31:51 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/327b44552e6f
Comment 24 Matt Brubeck (:mbrubeck) 2011-10-07 12:44:37 PDT
https://hg.mozilla.org/mozilla-central/rev/22f7450fdc4b
Comment 25 Camelia Urian 2011-10-21 02:27:35 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.