456870 - Fennec Addons UI button on right panel acts differently than all other buttons

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Al Billings [:abillings - ex-MoCo]]

On the right or left panels, when a user uses one of the buttons for an action (new tab, forward, backward, add bookmark), the panel immediately dismisses as soon as the button is used. Even when you do a two step action, such as editing an existing bookmark, the panel will automatically close as soon as the action is done. For the Addons UI button on the right panel, the panel does not close after the button is used, unlike all others.

This was found in the A8 build (20080923171103).

Steps to Reproduce
1. Open the right panel.
2. Select the Addons button and watch the Addons Browser window slide into view. 
3. Select the Addons button again and watch the Addons Browser window slide out of view.

Expected Result: Right panel should close.
Actual Result: Right panel is still open.

Comment 1

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

Let's not call that the "Addons button", since there is another button that specifically displays the add-ons manager.

Let's call it the "gear button" for now :)

Comment 2

[Madhava Enros [:madhava]]

This is actually more or less as intended, but I agree that the way to dismiss the "further UI" is not yet clear.  For one thing, the "gear" button should remain visibly depressed while the further UI is on screen.  I'm also thinking about greying out or disabling the sections of the control strip that are relevant to the webpage, not these browser options, so that there's a clearer separation of funciton.

Comment 3

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

The "gear" button is now visibly pressed while the panel is open

Comment 4

[Al Billings [:abillings - ex-MoCo]]

I guess this was fixed by bug 462771?

Comment 5

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

We are changing the way we display the right side panel. It will now display over the web content. It will not pan into view. Pressing the button will make the panel display over the webcontent. Pressing the button again will hide the panel.

Comment 6

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

Attachment: patch

First patch. Not 100% finished, but works well enough to review/land separately. We still need some new images for the buttons. Madhava is working on that.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 352241 [details] [diff] [review]
patch

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

>+    <!-- <spacer style="-moz-stack-sizing: ignore; width: 1px; height: 1px;" barriertype="vertical" size="10" left="881" constraint="vp-relative"/> -->
>+    <hbox id="panel-container" hidden="true" style="-moz-stack-sizing: ignore; width: 600px; height: 480px;" top="0" left="0">

Can we just remove the spacer? Can remove width/height too.

>+        <toolbarbutton id="tool-panel-close" class="browser-control-button" command="cmd_panel"/>

Shouldn't this be class="panel-button" like the others?

Comment 8

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

(In reply to comment #7)
> 
> Can we just remove the spacer? Can remove width/height too.

Done

> 
> >+        <toolbarbutton id="tool-panel-close" class="browser-control-button" command="cmd_panel"/>
> 
> Shouldn't this be class="panel-button" like the others?

Not until we get a button image in the panel-buttons.png

Comment 9

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

http://hg.mozilla.org/mobile-browser/rev/1920bad68d26

Comment 10

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

Madhava - To complete this we need the new images we talked about for the "gear" button and we need to change the dark gray background color for the addons, perfs, and download buttons.

reopening this bug until we get the images landed

Comment 12

[Madhava Enros [:madhava]]

Attachment: image with the new settings button

Some new images, to try out the concept of a button that goes beyond the page edge, and also to get all the browser-tools buttons on the same lighter background colour as the main chrome.


Comments from Sean:

1. created the new settings tab (settings.png) which has the default, pressed and disabled states for "settings" and "return".  I put all the states for settings/back, but would we actually see pressed or disabled since the screen shifts?  I've taken into account the width of the button all the way to the right edge of the browser, based on the toolbar's current width and the other button placements. For this to work in the panel screen, the width of the panel toolbar should match the default toolbar so the icons align.

2. I cropped right_buttons.png to remove "settings" from it.

3. I set panel_buttons.png background color to match the other buttons so it will sit properly on the lightened panel background.

Comment 13

[Madhava Enros [:madhava]]

Attachment: revised control-strip button set (removes the settings button, now in the settings button set)

Comment 14

[Madhava Enros [:madhava]]

Attachment: browser tools buttons, on the lighter background

Comment 15

[Madhava Enros [:madhava]]

Attachment: background for the toolbar

Comment 16

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

Attachment: added new buttons and background color

This patch adds the new provided buttons and backgrounds.

Comment 17

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

gavin: review ping

Comment 18

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

http://hg.mozilla.org/mobile-browser/rev/0e579a4e006f

Comment 19

[Madhava Enros [:madhava]]

two things:

1. The buttons don't visibly depress -- i.e. they don't change on mousedown. They don't feel like buttons, as a result.

2. I'd like to try having the [ <- ] end of the button, on the browser-tools toolbar, be visibly depressed when the user is in the browser tools section.  In other words, because it's "really" one button that extends into both sections of the browser (control strip and browser-tools toolbar), it should be pressed in while you're in the area that it takes you to.  You tap it again to un-depress it to take you back to the other area.

Comment 20

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

Attachment: handles button pressed look

simple css change for pressed looks of buttons. also <- button is pressed by default

Comment 21

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

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

Comment 22

[Al Billings [:abillings - ex-MoCo]]

Verified fixed with Mozilla/5.0 (X11; U; Linux a mv6l; en-US; rv:1.9.2a1pre) Gecko/20081223 Fennec/1.0a2.