Last Comment Bug 609107 - [Windows] Update dropmarkers for main window.
: [Windows] Update dropmarkers for main window.
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Theme (show other bugs)
: Trunk
: All Windows 7
: -- normal with 3 votes (vote)
: Firefox 13
Assigned To: tymerkaev
:
: Dão Gottwald [:dao]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-11-02 13:51 PDT by tymerkaev
Modified: 2013-12-27 14:31 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (10.38 KB, patch)
2010-11-03 11:37 PDT, tymerkaev
no flags Details | Diff | Splinter Review
patch v2 (10.64 KB, patch)
2010-11-03 12:16 PDT, tymerkaev
dao+bmo: review-
Details | Diff | Splinter Review
patch v3 (11.46 KB, patch)
2010-11-06 06:36 PDT, tymerkaev
dao+bmo: review-
Details | Diff | Splinter Review
patch v4 (8.15 KB, patch)
2011-02-25 06:34 PST, tymerkaev
no flags Details | Diff | Splinter Review
updated patch (10.89 KB, patch)
2012-02-09 08:22 PST, Dão Gottwald [:dao]
dao+bmo: review+
Details | Diff | Splinter Review

Description tymerkaev 2010-11-02 13:51:42 PDT

    
Comment 1 tymerkaev 2010-11-03 11:37:43 PDT
Created attachment 487969 [details] [diff] [review]
patch
Comment 2 tymerkaev 2010-11-03 12:16:20 PDT
Created attachment 487981 [details] [diff] [review]
patch v2
Comment 3 Dão Gottwald [:dao] 2010-11-04 05:34:17 PDT
Comment on attachment 487981 [details] [diff] [review]
patch v2

>-.toolbarbutton-1 > .toolbarbutton-menubutton-dropmarker,
> toolbar[iconsize="small"][mode="icons"] .toolbarbutton-1 > .toolbarbutton-menubutton-button,
> toolbar[iconsize="small"][mode="icons"] .toolbarbutton-1 {
>   padding-left: 3px;
>   padding-right: 3px;
> }
> 
>+.toolbarbutton-1 > .toolbarbutton-menubutton-dropmarker,
>+.toolbarbutton-1 > .toolbarbutton-menu-dropmarker {
>+  list-style-image: url("chrome://browser/skin/dropdown-arrow.png");
>+  -moz-image-region: rect(0, 11px, 9px, 0);
>+  padding: 2px;
>+}
>+
>+.toolbarbutton-1 > .toolbarbutton-menu-dropmarker {
>+  -moz-padding-end: 0;
>+  margin: 0;
>+}

This appears to be making menu buttons much wider. That is, wider than they are in the mockups.

> .urlbar-history-dropmarker {
>   -moz-appearance: none;
>-  padding: 0 1px;
>+  padding: 2px;
>   background-color: transparent;
>   border: none;
>   width: auto;
>-  list-style-image: url(mainwindow-dropdown-arrow.png);
>-  -moz-image-region: rect(0, 13px, 11px, 0);
>+  list-style-image: url("chrome://browser/skin/dropdown-arrow.png");
>+  -moz-image-region: rect(0, 11px, 9px, 0);
> }
> 
> .urlbar-history-dropmarker:-moz-system-metric(touch-enabled) {
>@@ -1154,7 +1165,7 @@ html|*.urlbar-input:-moz-lwtheme:-moz-pl
> 
> .urlbar-history-dropmarker:hover:active,
> .urlbar-history-dropmarker[open="true"] {
>-  -moz-image-region: rect(0, 26px, 11px, 13px);
>+  -moz-image-region: rect(0, 22px, 9px, 11px);
> }

Can you avoid this and focus on toolbarbuttons in this bug?

>--- a/toolkit/themes/winstripe/global/toolbarbutton.css
>+++ b/toolkit/themes/winstripe/global/toolbarbutton.css
>@@ -147,10 +147,6 @@ toolbarbutton[checked="true"]:not([disab
>   margin-top: 1px;
> }
> 
>-.toolbarbutton-menu-dropmarker[disabled="true"] {
>-  padding: 0 !important;
>-}

What's the story here about? Why can this be removed without some replacement?
Comment 4 tymerkaev 2010-11-06 06:36:28 PDT
Created attachment 488647 [details] [diff] [review]
patch v3

(In reply to comment #3)
> >-.toolbarbutton-1 > .toolbarbutton-menubutton-dropmarker,
> > toolbar[iconsize="small"][mode="icons"] .toolbarbutton-1 > .toolbarbutton-menubutton-button,
> > toolbar[iconsize="small"][mode="icons"] .toolbarbutton-1 {
> >   padding-left: 3px;
> >   padding-right: 3px;
> > }
> > 
> >+.toolbarbutton-1 > .toolbarbutton-menubutton-dropmarker,
> >+.toolbarbutton-1 > .toolbarbutton-menu-dropmarker {
> >+  list-style-image: url("chrome://browser/skin/dropdown-arrow.png");
> >+  -moz-image-region: rect(0, 11px, 9px, 0);
> >+  padding: 2px;
> >+}
> >+
> >+.toolbarbutton-1 > .toolbarbutton-menu-dropmarker {
> >+  -moz-padding-end: 0;
> >+  margin: 0;
> >+}
> 
> This appears to be making menu buttons much wider. That is, wider than they are in the mockups.

Fixed by removing extra glow from dropmarker itself.
> > .urlbar-history-dropmarker {
> >   -moz-appearance: none;
> >-  padding: 0 1px;
> >+  padding: 2px;
> >   background-color: transparent;
> >   border: none;
> >   width: auto;
> >-  list-style-image: url(mainwindow-dropdown-arrow.png);
> >-  -moz-image-region: rect(0, 13px, 11px, 0);
> >+  list-style-image: url("chrome://browser/skin/dropdown-arrow.png");
> >+  -moz-image-region: rect(0, 11px, 9px, 0);
> > }
> > 
> > .urlbar-history-dropmarker:-moz-system-metric(touch-enabled) {
> >@@ -1154,7 +1165,7 @@ html|*.urlbar-input:-moz-lwtheme:-moz-pl
> > 
> > .urlbar-history-dropmarker:hover:active,
> > .urlbar-history-dropmarker[open="true"] {
> >-  -moz-image-region: rect(0, 26px, 11px, 13px);
> >+  -moz-image-region: rect(0, 22px, 9px, 11px);
> > }
> 
> Can you avoid this and focus on toolbarbuttons in this bug?

No, .urlbar-history-dropmarker using mainwindow-dropdown-arrow.png, which should be replaced by new image.
> >-.toolbarbutton-menu-dropmarker[disabled="true"] {
> >-  padding: 0 !important;
> >-}
> 
> What's the story here about? Why can this be removed without some replacement?

Same as for http://hg.mozilla.org/mozilla-central/diff/a05ab26714f6/toolkit/themes/winstripe/global/toolbarbutton.css.
Comment 5 Dão Gottwald [:dao] 2010-11-06 07:18:52 PDT
Comment on attachment 488647 [details] [diff] [review]
patch v3

Using the new image for the location bar was neither planned nor does it look better, imho (feels heavier). Just avoid that change and name the new file toolbarbutton-dropdown-arrow.png.
Comment 6 imradyurrad 2010-11-29 23:24:10 PST
Does this also effect the dropdown marker in the bookmarks toolbar button?
Comment 7 Dão Gottwald [:dao] 2010-11-29 23:41:50 PST
(In reply to comment #6)
> Does this also effect the dropdown marker in the bookmarks toolbar button?

Yes.
Comment 8 tymerkaev 2011-02-25 06:34:13 PST
Created attachment 515058 [details] [diff] [review]
patch v4
Comment 9 Kurt Felton 2011-05-02 12:17:07 PDT
Is there something still holding this bug back? Just wondering what else needs to be done.
Comment 10 Asa Dotzler [:asa] 2011-07-03 11:44:11 PDT
Kurt, you can almost always answer these kinds of questions for yourself (like your question in bug 618353) If you look at the patch, you'll see that it's awaiting a review from Dão. 

It's a safe bet that Dão has not forgotten about this bug but he's got a rather large set of responsibilities. He has a good track record of prioritizing them well so I don't think that there's much value in "are we there yet" comments or other "pokes".
Comment 11 Dão Gottwald [:dao] 2012-02-09 08:22:52 PST
Created attachment 595759 [details] [diff] [review]
updated patch

Updated to tip, renamed a few files, removed a few changes not needed for this bug (e.g. the menu-vertical binding and the change to the search engine button's dropmarker). Patch is good to go with these modifications.
Comment 13 Ed Morley [:emorley] 2012-02-10 05:07:42 PST
https://hg.mozilla.org/mozilla-central/rev/401146857eb9
Comment 14 Kurt Felton 2012-02-12 00:06:36 PST
Is this fixed completely? On nightly, noticed the dropdown arrow for Add-ons Manager section next to the wrench icon still has the old dropdown arrow.

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