614091 - having 4 or more tabs, zooming in and switching to portrait to landscape will show part of the tab side panel

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Mozilla/5.0 (Android; Linux armv71; rv2.0b8pre) Gecko/20101122 Firefox/4.0b8pre Fennec/4.0b3pre
Mozilla/5.0 (Maemo; Linux armv71; rv:2.0b8pre) Gecko/20101122 Firefox/4.0b8pre Fennec/4.0b3pre

1. close the hardware keyboard
2. go to portrait mode
3. open at least 4 tabs
4. go to http://people.mozilla.com/~nhirata/html_tp/htmllinks_test.html
5. zoom in
6. go to the 0,0 corner of the content 
7. switch to landscape

Expected: Side panel does not show
Actual: part of the side panel shows

Note:
1. control side panel may show when switching from landscape to portrait

Comment 1

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Attachment: screenshot

Comment 2

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

Attachment: Patch + tests

The patch fix those case for both LTR/RTL. Also I've added some tests to ensure we dont regress anymore on that.

Mark, I'm not sure the tests will work on n900 since I'm using some window.resizeTo calls? If they won't I would like to keep those tests but turn them off on device if possible?

Comment 4

[Aakash Desai [:aakashd]]

Seen in the wild: http://input.mozilla.com/en-US/opinion/765310 Vivien says that the patch associated to this bug should help some other general "jumpiness" bugs in regards to form assistant.

Comment 5

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

Comment on attachment 492664 [details] [diff] [review]
Patch + tests

>diff --git a/chrome/content/browser.js b/chrome/content/browser.js

>-        Browser.controlsScrollboxScroller.scrollTo(Browser.controlsPosition.x, 0);
>-        Browser.pageScrollboxScroller.scrollTo(0, Browser.controlsPosition.y);
>+        // Handle Width transformation of the tabs sidebar
>+        if (restorePosition.x) {
>+          let [,, leftWidth, rightWidth] = Browser.computeSidebarVisibility();
>+          let delta = ((restorePosition.leftSidebar - leftWidth) || (restorePosition.rightSidebar - rightWidth));
>+          restorePosition.x  += (restorePosition.x == leftWidth) ? delta : -delta;

            restorePosition.x += (restorePosition.x == leftWidth) ? delta : -delta;

Comment 6

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

http://hg.mozilla.org/mobile-browser/rev/11caeb1c9958

Comment 7

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

Attachment: patch followup

Since the typo fix from heigth -> height the tab size is wrong on start unti you get a second column. The patch resolve this by moving to 0 at the beginning to enter updateWidth and have the 20px applying. This does not change anything for startup since the columnsCount was undefined before and we were entering this code path too.

Comment 8

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

Comment on attachment 497936 [details] [diff] [review]
patch followup

Oups, I've forgot to ask r? for this littte change

Comment 9

[Aakash Desai [:aakashd]]

verified FIXED on build:
Mozilla/5.0 (Android; Linux armv71; rv:2.0b9pre) Gecko/20101227 Namoroka/4.0b9pre Fennec/4.0b4pre