Closed
Bug 1168528
Opened 9 years ago
Closed 9 years ago
Tab close icons on Windows HiDPI are blurry
Categories
(Firefox :: Theme, defect)
Firefox
Theme
Tracking
()
People
(Reporter: jaws, Assigned: jaws)
References
Details
Attachments
(3 files, 3 obsolete files)
8.55 KB,
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
13.96 KB,
patch
|
jaws
:
review+
|
Details | Diff | Splinter Review |
22.31 KB,
patch
|
lmandel
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
No description provided.
Flags: qe-verify-
Flags: firefox-backlog+
Assignee | ||
Updated•9 years ago
|
status-firefox41:
affected → ---
Assignee | ||
Comment 1•9 years ago
|
||
toolkit/themes/windows/global/icons/close-win8.png was unused. Stephen said over IRC[1] that we should use the Windows8 close icons for all versions of Windows, so I've overwritten close.png with close-win8.png and used close-win8@2x.png from the icons attachment in bug 1147702 for close@2x.png. [1] http://logs.glob.uno/?c=mozilla%23fx-team#c207859
Attachment #8610786 -
Flags: review?(gijskruitbosch+bugs)
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8610805 -
Flags: review?(gijskruitbosch+bugs)
Assignee | ||
Updated•9 years ago
|
Attachment #8610786 -
Flags: review?(gijskruitbosch+bugs)
Assignee | ||
Comment 3•9 years ago
|
||
Attachment #8610786 -
Attachment is obsolete: true
Comment 4•9 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #1) > Stephen said over IRC[1] that we should use the Windows8 close icons for all > versions of Windows, so I've overwritten close.png with close-win8.png and > used close-win8@2x.png from the icons attachment in bug 1147702 for > close@2x.png. Even better would be to switch to SVG like Linux uses…
Comment 5•9 years ago
|
||
Comment on attachment 8610815 [details] [diff] [review] Patch, part 1 (close icons) Review of attachment 8610815 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/themes/windows/global/global.css @@ -336,5 @@ > .close-icon:hover:active { > -moz-image-region: rect(0, 48px, 16px, 32px); > } > > -%ifdef XP_WIN You should keep this, as Linux is already using SVG...
Comment 6•9 years ago
|
||
Comment on attachment 8610805 [details] [diff] [review] Patch, part 2 (new tab and tab arrows) Review of attachment 8610805 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/themes/windows/browser.css @@ +1973,5 @@ > + #TabsToolbar > toolbarpaletteitem > #new-tab-button { > + list-style-image: url(chrome://browser/skin/tabbrowser/newtab@2x.png); > + } > + > + .tabs-newtab-button > .toolbarbutton-icon { I would have expected this rule to need to include the other two selectors, too. Am I wrong? If so, please add a comment to the code as to why.
Attachment #8610805 -
Flags: review?(gijskruitbosch+bugs) → review+
Assignee | ||
Updated•9 years ago
|
Attachment #8610815 -
Flags: review?(gijskruitbosch+bugs)
Assignee | ||
Updated•9 years ago
|
Attachment #8610815 -
Flags: review?(gijskruitbosch+bugs)
Assignee | ||
Comment 7•9 years ago
|
||
Added the %ifdef guards around the .close-icon section in global.css. Note that I removed the %ifdef in the previous version because it was only guarding the media queries for lunaBlue, etc.
Attachment #8610815 -
Attachment is obsolete: true
Attachment #8611326 -
Flags: review?(gijskruitbosch+bugs)
Assignee | ||
Comment 8•9 years ago
|
||
Added the extra selectors to the rule. Previously, there was a couple pixel difference between the new tab button as a tab, the docked new tab button, and the new tab button in customization mode. With this change, they are now all the same size.
Attachment #8610805 -
Attachment is obsolete: true
Attachment #8611327 -
Flags: review+
Comment 9•9 years ago
|
||
Comment on attachment 8611326 [details] [diff] [review] Patch, part 1 v1.1 Review of attachment 8611326 [details] [diff] [review]: ----------------------------------------------------------------- r=me but see below. ::: toolkit/themes/windows/global/global.css @@ +341,5 @@ > +@media (min-resolution: 1.1dppx) { > + .close-icon > .button-icon, > + .close-icon > .button-box > .button-icon, > + .close-icon > .toolbarbutton-icon { > + width: 16px; Should we do this everywhere, not just for hidpi, for consistency and so we can spot issues more quickly? This might also affect add-ons.
Attachment #8611326 -
Flags: review?(gijskruitbosch+bugs) → review+
Comment 10•9 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #7) > Created attachment 8611326 [details] [diff] [review] > Patch, part 1 v1.1 > > Added the %ifdef guards around the .close-icon section in global.css. Note > that I removed the %ifdef in the previous version because it was only > guarding the media queries for lunaBlue, etc. Huh. Rereading code and doublechecking jar.mn, AFAICT the linux one overrides the Windows one anyway, so we can indeed do without the %ifdef. Sorry about leading you astray there. I also suspect (but would like shorlander/mmaslaney/some-graphics-y-person to doublecheck) that we could use the linux SVGs on Windows (potentially with minor tweaking). Can you file a followup for that?
Comment 11•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/634e5b445a57 https://hg.mozilla.org/integration/fx-team/rev/028160c9d05c
Comment 13•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/634e5b445a57 https://hg.mozilla.org/mozilla-central/rev/028160c9d05c https://hg.mozilla.org/mozilla-central/rev/d93f6b15c003
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Assignee | ||
Comment 14•9 years ago
|
||
Approval Request Comment [Feature/regressing bug #]: Windows 10 HiDPI [User impact if declined]: HiDPI icons on Windows are blurry [Describe test coverage new/current, TreeHerder]: on mozilla-central for much of 41-nightly [Risks and why]: none expected [String/UUID change made/needed]: none https://hg.mozilla.org/try/pushloghtml?changeset=71084a9edf1e
Attachment #8627204 -
Flags: approval-mozilla-beta?
Updated•9 years ago
|
status-firefox40:
--- → affected
Comment 15•9 years ago
|
||
Comment on attachment 8627204 [details] [diff] [review] Patch for 40 Visual fix in support of Windows 10. On m-c for more than a month. Beta+
Attachment #8627204 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in
before you can comment on or make changes to this bug.
Description
•