523736 - modify sidebar behavior

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Madhava Enros [:madhava]]

Two cases where sidebars were left out by the user but should be "put away" automatically:

1. When a user goes to a new page from the awesomescreen (by tapping on a row or entering a url or search).  They're going somewhere new, which should get full-screen attention, but also, because the awesomescreen has taken up the whole screen, it's not jarring that the sidebars are gone afterwards.

2. When a user taps on a "Go to page" button in one of the managers.  We should get the right (controls) sidebar out of the way.  The rationale is the same as for point 1 above.  This is different from when the user closes the browser tools section manually in that, in that case, we want to take them back out to where they came in.

I think this will fix a lot of what feels odd about sidebar behavior at the moment.  It takes care of the new-tab case because opening a new tab is followed by going somewhere in the awesomebar; when you're returned to the content area after tapping on a row, the sidebar is out of the way.

Comment 1

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

> I think this will fix a lot of what feels odd about sidebar behavior at the
> moment.  It takes care of the new-tab case because opening a new tab is
> followed by going somewhere in the awesomebar; when you're returned to the
> content area after tapping on a row, the sidebar is out of the way.

Madhava, what do you want in the case where the user create a new tab and go dismiss the awesome screen without going to a new page, should we still show the sidebars or not?

Comment 2

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

(In reply to comment #1)

> Madhava, what do you want in the case where the user create a new tab and go
> dismiss the awesome screen without going to a new page, should we still show
> the sidebars or not?

Basically, we will be hiding the sidebars when we start loading a page. So if the user cancels the awesomebar, nothing happens. However, since we do not want to hide the sidebar when a back or forward happens, I think we need to put the "hide sidebars" code in the networkStart method, since that isn't called when going back/forward.

Note: adding the "hide sidebar" code to networkStart will fix both issues listed in comment #0

Comment 3

[Benjamin Stover (:stechz)]

> Basically, we will be hiding the sidebars when we start loading a page.

What about when the user clicks a link and a sidebar is visible?  Or the page reloads itself?

Comment 4

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

Attachment: Patch

Basically, the patch hide the sidebars when we enter the awesome bar and when a new tab is created, which covers madhava's requirements, but not every time a page load because we don't want the sidebars to hide when a page is refreshed every seconds (otherwise I think a website can own us by forcing the sidebars to hide forever).

Also, I'm not sure that following a link should hide the sidebars, because again, a web site can own us by forcing some network start calls (refreshing an iframe, emulating a click on a download link, ..)

Each time I've tried to put hideSidebars late in the process to avoid seeing the content to move to the left/right just before opening the awesome bar / creating a new tab.

Comment 5

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

Comment on attachment 409008 [details] [diff] [review]
Patch

I think we want to hideSidebars in BrowserUI.goToURI() - we use that for any awesomebar or bookmark selection and if the user types into the urlbar and presses "go" or vk_enter.

Adding to BrowserUI.newTab is a problem because the new tab button (command) uses it and the add-on mgr and download mgr use it.

We can't always call hideSidebars in BrowserUI.newTab(). I guess we could only call it if the aURI is not "about:blank"

Comment 6

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

Attachment: Patch v0.2

After have seen on IRC with mfinkle I think I should never try to think again :)

Basically the patch do the same things but don't hide the sidebars if we manually dismiss the awesomebar panel

Comment 7

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

pushed:
https://hg.mozilla.org/mobile-browser/rev/a925a8fe0f31

Comment 8

[Aakash Desai [:aakashd]]

verified FIXED on tinderbox build:

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2b2pre) Gecko/20091029 Namoroka/3.6b2pre Fennec/1.0b5

Comment 9

[Aakash Desai [:aakashd]]

litmus testcases:

https://litmus.mozilla.org/show_test.cgi?id=11804
https://litmus.mozilla.org/show_test.cgi?id=11805

created to regression test this bug.