Closed
Bug 885052
Opened 11 years ago
Closed 11 years ago
Fullscreen-button can has pressed state in customization mode
Categories
(Firefox :: Toolbars and Customization, defect)
Firefox
Toolbars and Customization
Tracking
()
RESOLVED
FIXED
Firefox 29
People
(Reporter: jaws, Assigned: jaws)
References
(Blocks 1 open bug)
Details
(Whiteboard: [Australis:P4])
Attachments
(1 file)
(deleted),
patch
|
Unfocused
:
review+
aks
:
feedback+
|
Details | Diff | Splinter Review |
No description provided.
Updated•11 years ago
|
Whiteboard: [Australis:M?] → [Australis:M?][Australis:P4]
Assignee | ||
Comment 1•11 years ago
|
||
To fix this bug, you'll need to clone the repository at https://hg.mozilla.org/projects/ux/.
We'll probably want to add to the selector that applies the pressed-in/active state so that it isn't applied when #main-window[customizing] is true.
Whiteboard: [Australis:M?][Australis:P4] → [Australis:M?][Australis:P4][good-first-bug][mentor=jaws][lang=css]
Updated•11 years ago
|
Whiteboard: [Australis:M?][Australis:P4][good-first-bug][mentor=jaws][lang=css] → [Australis:M?][Australis:P4][good first bug][mentor=jaws][lang=css]
Assignee | ||
Comment 2•11 years ago
|
||
STR:
1) Open a build of Firefox running Australis.
2) Click on the menu button and then click on the Full Screen button
3) Move your mouse to the top of the screen to see the Navigation Toolbar reappear
4) Click on the menu button
5) Notice that the Full Screen button is shown as active, since you are in full screen.
6) Click on Customize
7) Notice that the Full Screen button is still shown as active.
We don't want buttons in customize mode to resemble current state of the browser, as this mode should feel detached from what else is going on in the browser at the moment.
Comment 3•11 years ago
|
||
Hey I am interested in this bug.
How can I help ?
I am new here !
Assignee | ||
Comment 4•11 years ago
|
||
Hi Akshat, I have provided some steps for you in comment #1. Please let me know if I should explain more.
Assignee: nobody → aksht.kedia
Status: NEW → ASSIGNED
Assignee | ||
Comment 5•11 years ago
|
||
Any update on this? Let me know if there is anything I can do to help.
Flags: needinfo?(aksht.kedia)
Comment 6•11 years ago
|
||
Can you give me a bit more of information about the bug ? I do get what exactly is happening but not really understand what I have to implement.
Flags: needinfo?(aksht.kedia)
Assignee | ||
Comment 7•11 years ago
|
||
(In reply to Akshat Kedia from comment #6)
> Can you give me a bit more of information about the bug ? I do get what
> exactly is happening but not really understand what I have to implement.
When customization mode is entered, we should stop displaying the full screen button as though it is active/enabled.
We will probably need to adjust some CSS that checks for [checked="true"] so that it doesn't apply if #main-window[customizing] is present.
For example, from http://mxr.mozilla.org/mozilla-central/source/browser/themes/osx/browser.css#1032 we could do the following:
> #fullscreen-button {
> -moz-image-region: rect(0, 340px, 20px, 320px);
> }
>
> -#fullscreen-button[checked="true"],
> +#main-window[customizing] #fullscreen-button[checked="true"],
> #restore-button {
> -moz-image-region: rect(0, 360px, 20px, 340px);
> }
I think the Windows style is implemented at http://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/browser.css#893 but that is a much more generic rule and might not be so easy to work around. A hacky way to do it would be the following:
> @navbarLargeIcons@ .toolbarbutton-1 > .toolbarbutton-menubutton-button:not([disabled]):hover:active > .toolbarbutton-icon,
> @navbarLargeIcons@ .toolbarbutton-1[open] > .toolbarbutton-menubutton-dropmarker:not([disabled]) > .dropmarker-icon,
> -@navbarLargeIcons@ .toolbarbutton-1:not([disabled]):-moz-any([open],[checked],:hover:active) > .toolbarbutton-icon,
> +@navbarLargeIcons@ .toolbarbutton-1:not([disabled]):not(#fullscreen-button):-moz-any([open],[checked],:hover:active) > .toolbarbutton-icon,
> +#main-window:not([customizing]) @navbarLargeIcons@ #fullscreen-button:not([disabled]):-moz-any([open],[checked],:hover:active) > .toolbarbutton-icon,
> @navbarLargeIcons@ .toolbarbutton-1:not([disabled]):-moz-any([open],[checked],:hover:active) > .toolbarbutton-badge-container {
> background-image: linear-gradient(hsla(0,0%,100%,.6), hsla(0,0%,100%,.1));
> background-color: hsla(210,54%,20%,.15);
> border-color: hsla(210,54%,20%,.3) hsla(210,54%,20%,.35) hsla(210,54%,20%,.4);
> box-shadow: 0 1px 1px hsla(210,54%,20%,.1) inset,
> 0 0 1px hsla(210,54%,20%,.2) inset,
> /* allows windows-keyhole-forward-clip-path to be used for non-hover as well as hover: */
> 0 1px 0 hsla(210,54%,20%,0),
> 0 0 2px hsla(210,54%,20%,0);
> text-shadow: none;
> transition: none;
> }
Although IMO it's kinda ugly, off the top of my head I'm not sure of a better solution for Windows. I hope that helps.
Comment 8•11 years ago
|
||
Can you please tell me what does this CSS exactly do ?
#fullscreen-button[customizableui-areatype="toolbar"] {
list-style-image: url("moz-icon://stock/gtk-fullscreen?size=toolbar");
}
Comment 9•11 years ago
|
||
(In reply to Akshat Kedia from comment #8)
> Can you please tell me what does this CSS exactly do ?
>
> #fullscreen-button[customizableui-areatype="toolbar"] {
> list-style-image: url("moz-icon://stock/gtk-fullscreen?size=toolbar");
> }
It sets the icon for the fullscreen button to url("moz-icon://stock/gtk-fullscreen?size=toolbar") when the button is in the toolbar. That url("moz-icon...") thing points to the stock GTK fullscreen icon. Does that help?
Comment 10•11 years ago
|
||
Any updates on this bug, Akshat? Are you still working on it?
Flags: needinfo?(aksht.kedia)
Comment 11•11 years ago
|
||
I am working on a linux environment. Can you please help me with the implementation here ?
Flags: needinfo?(aksht.kedia) → needinfo?(jaws)
Comment 13•11 years ago
|
||
I tried the above windows implementation but it does not seem to fix the bug(I used a windows machine). I am still able to reproduce the bug.
Flags: needinfo?(jaws)
Assignee | ||
Comment 14•11 years ago
|
||
I'm sorry Akshat, the bug turned out to be quite different than I was expecting. I've put up a patch here that fixes the issue and includes a test that reproduces the issue and ensures that it won't break in the future. Please let me know what you think of it.
Assignee: aksht.kedia → jaws
Attachment #8348461 -
Flags: review?(bmcbride)
Attachment #8348461 -
Flags: feedback?(aksht.kedia)
Assignee | ||
Updated•11 years ago
|
Flags: needinfo?(jaws)
Whiteboard: [Australis:M?][Australis:P4][good first bug][mentor=jaws][lang=css] → [Australis:P4]
Updated•11 years ago
|
Attachment #8348461 -
Flags: review?(bmcbride) → review+
Assignee | ||
Comment 15•11 years ago
|
||
Flags: in-testsuite+
Assignee | ||
Comment 16•11 years ago
|
||
Backed out to reland using add_task,
https://hg.mozilla.org/integration/fx-team/rev/6969b6e1f90b
Assignee | ||
Comment 17•11 years ago
|
||
Comment 18•11 years ago
|
||
(In reply to Jared Wein [:jaws] (Away 20 Dec to 2 Jan) from comment #17)
> https://hg.mozilla.org/integration/fx-team/rev/af77b8d3a9e1
hey Jared, had to backout this again for testfailures like https://tbpl.mozilla.org/php/getParsedLog.php?id=32074966&tree=Fx-Team
Assignee | ||
Comment 19•11 years ago
|
||
Made some tweaks to the test and confirmed that it would work on Linux. Repushed,
https://hg.mozilla.org/integration/fx-team/rev/98aa3895f04d
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
Comment 21•11 years ago
|
||
Comment on attachment 8348461 [details] [diff] [review]
Patch
I had sort of started getting an idea that we were looking at the wrong place. Great this bug is fixed now :D Although, I didnt really understand how you fixed it.
Attachment #8348461 -
Flags: feedback?(aksht.kedia) → feedback+
Comment 22•11 years ago
|
||
This test is still causing frequent random orange, see bug 951403. :-(
Depends on: 951403
Assignee | ||
Comment 23•11 years ago
|
||
(In reply to :Gijs Kruitbosch (Unavailable Dec 19 - Jan 2) from comment #22)
> This test is still causing frequent random orange, see bug 951403. :-(
The patch in bug 933926 includes the potential fix for this intermittent orange. I'll see about separating it out so it can land sooner.
You need to log in
before you can comment on or make changes to this bug.
Description
•