Closed
Bug 1023607
Opened 11 years ago
Closed 10 years ago
Implement Windows HiDPI theme for other icons and tiny widgets
Categories
(Firefox :: Theme, defect)
Tracking
()
People
(Reporter: Dolske, Assigned: jaws)
References
Details
Attachments
(3 files, 1 obsolete file)
3.70 KB,
patch
|
Details | Diff | Splinter Review | |
62.09 KB,
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
62.10 KB,
patch
|
lmandel
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This is a somewhat arbitrary breakdown of bug 1023511.
Part of the OS X Retina work (in bug 781327 and bug 817366) was adding HiDPI versions of various random icons and tiny widgets, which don't really have a better-defined grouping. See those bugs for details of which icons. We may also want to just look through the Windows theme dirs, and fix a big batch of such random icons here.
This is a somewhat loose "breakdown", since there's a bunch of these because we are basically have no existing Windows HiDPI assets. I'd suggest we just try to fix a big batch of them in this pass, and then do a separate pass to do remaining work in detail. (As opposed to trying to be perfect and complete all in one shot.)
Flags: firefox-backlog+
Comment 1•10 years ago
|
||
Do we have the necessary UX assets to start implementing this?
Points: --- → 8
Assignee | ||
Comment 2•10 years ago
|
||
This bug can probably pick up things like the stop/reload icons, the search icon, and the awesome bar dropdown arrow.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Iteration: --- → 41.1 - May 25
Flags: qe-verify+
Assignee | ||
Comment 3•10 years ago
|
||
This patch adds hidpi icons for the identity icons, stop/reload/go buttons, urlbar dropdown, customize mode titlebar button, and notification arrow in urlbar.
Still need hidpi notification icons.
Attachment #8609629 -
Flags: review?(gijskruitbosch+bugs)
Comment 4•10 years ago
|
||
Comment on attachment 8609629 [details] [diff] [review]
Patch
Review of attachment 8609629 [details] [diff] [review]:
-----------------------------------------------------------------
Almost there. We should probably include the urlbar- things on Linux, too, for consistency. There's a couple of places where we are (or the patch starts) using 2dppx instead of 1.1dppx which should be updated.
::: browser/themes/shared/notification-icons.inc.css
@@ +316,1 @@
> @media (min-resolution: 2dppx) {
2dppx instead of 1.1 ?
::: browser/themes/windows/browser.css
@@ +1400,5 @@
> background-image: radial-gradient(circle closest-side, hsla(205,100%,70%,.1), transparent);
> -moz-image-region: var(--urlbar-dropmarker-active-region);
> }
>
> +@media (min-resolution: 2dppx) {
1.1dppx
::: browser/themes/windows/jar.mn
@@ +137,5 @@
> skin/classic/browser/undoCloseTab.png (../shared/undoCloseTab.png)
> skin/classic/browser/undoCloseTab@2x.png (../shared/undoCloseTab@2x.png)
> skin/classic/browser/update-badge.svg (../shared/update-badge.svg)
> skin/classic/browser/urlbar-arrow.png
> + skin/classic/browser/urlbar-arrow@2x.png
Why is this not on Linux as well?
@@ +142,3 @@
> skin/classic/browser/urlbar-popup-blocked.png
> skin/classic/browser/urlbar-history-dropmarker.png
> + skin/classic/browser/urlbar-history-dropmarker@2x.png
Ditto
Attachment #8609629 -
Flags: review?(gijskruitbosch+bugs) → feedback+
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #8609629 -
Attachment is obsolete: true
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #4)
> ::: browser/themes/shared/notification-icons.inc.css
> @@ +316,1 @@
> > @media (min-resolution: 2dppx) {
>
> 2dppx instead of 1.1 ?
Fixed.
> ::: browser/themes/windows/browser.css
> @@ +1400,5 @@
> > background-image: radial-gradient(circle closest-side, hsla(205,100%,70%,.1), transparent);
> > -moz-image-region: var(--urlbar-dropmarker-active-region);
> > }
> >
> > +@media (min-resolution: 2dppx) {
>
> 1.1dppx
Fixed
> ::: browser/themes/windows/jar.mn
> @@ +137,5 @@
> > skin/classic/browser/undoCloseTab.png (../shared/undoCloseTab.png)
> > skin/classic/browser/undoCloseTab@2x.png (../shared/undoCloseTab@2x.png)
> > skin/classic/browser/update-badge.svg (../shared/update-badge.svg)
> > skin/classic/browser/urlbar-arrow.png
> > + skin/classic/browser/urlbar-arrow@2x.png
>
> Why is this not on Linux as well?
Fixed.
> @@ +142,3 @@
> > skin/classic/browser/urlbar-popup-blocked.png
> > skin/classic/browser/urlbar-history-dropmarker.png
> > + skin/classic/browser/urlbar-history-dropmarker@2x.png
>
> Ditto
This is not used on Linux. Linux uses -moz-appearance: toolbarbutton-dropdown. We can probably change Linux to use this custom styling, but I don't think this is the right bug for that.
Attachment #8610736 -
Flags: review?(gijskruitbosch+bugs)
Comment 7•10 years ago
|
||
Comment on attachment 8610736 [details] [diff] [review]
Patch v1.1
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6)
> > @@ +142,3 @@
> > > skin/classic/browser/urlbar-popup-blocked.png
> > > skin/classic/browser/urlbar-history-dropmarker.png
> > > + skin/classic/browser/urlbar-history-dropmarker@2x.png
> >
> > Ditto
>
> This is not used on Linux. Linux uses -moz-appearance:
> toolbarbutton-dropdown. We can probably change Linux to use this custom
> styling, but I don't think this is the right bug for that.
For sure.
Attachment #8610736 -
Flags: review?(gijskruitbosch+bugs) → review+
Assignee | ||
Updated•10 years ago
|
Iteration: 41.1 - May 25 → 41.2 - Jun 8
Comment 9•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Comment 10•10 years ago
|
||
Why the Linux changes? Were you aware that we don't have support for using this stuff on Linux?
Flags: needinfo?(jaws)
Assignee | ||
Comment 11•10 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #10)
> Why the Linux changes? Were you aware that we don't have support for using
> this stuff on Linux?
I wasn't aware. I admit that having untested/unusable code in the tree is generally unwelcome, but do you think when we have support for this that much will need to be changed?
Flags: needinfo?(jaws) → needinfo?(dao)
Comment 12•10 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #11)
> (In reply to Dão Gottwald [:dao] from comment #10)
> > Why the Linux changes? Were you aware that we don't have support for using
> > this stuff on Linux?
>
> I wasn't aware. I admit that having untested/unusable code in the tree is
> generally unwelcome, but do you think when we have support for this that
> much will need to be changed?
It's unclear when we'll have support, and as you said the styles being untested makes it more likely that they'll rot over time.
Flags: needinfo?(dao)
Updated•10 years ago
|
QA Contact: cornel.ionce
Comment 13•10 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #12)
> (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #11)
> > (In reply to Dão Gottwald [:dao] from comment #10)
> > > Why the Linux changes? Were you aware that we don't have support for using
> > > this stuff on Linux?
> >
> > I wasn't aware. I admit that having untested/unusable code in the tree is
> > generally unwelcome, but do you think when we have support for this that
> > much will need to be changed?
>
> It's unclear when we'll have support, and as you said the styles being
> untested makes it more likely that they'll rot over time.
Just for my curiosity, what bit isn't supported? I'm almost sure I've seen people experiment with this by setting the relevant dpi prefs to something other than -1 by themselves in the past. So do you just mean automatic DPI/resolution detection? That seems to have been fixed for 38 by https://bugzilla.mozilla.org/show_bug.cgi?id=1031073 . Am I missing something?
Flags: needinfo?(dao)
Comment 14•10 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #13)
> (In reply to Dão Gottwald [:dao] from comment #12)
> > (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #11)
> > > (In reply to Dão Gottwald [:dao] from comment #10)
> > > > Why the Linux changes? Were you aware that we don't have support for using
> > > > this stuff on Linux?
> > >
> > > I wasn't aware. I admit that having untested/unusable code in the tree is
> > > generally unwelcome, but do you think when we have support for this that
> > > much will need to be changed?
> >
> > It's unclear when we'll have support, and as you said the styles being
> > untested makes it more likely that they'll rot over time.
>
> Just for my curiosity, what bit isn't supported? I'm almost sure I've seen
> people experiment with this by setting the relevant dpi prefs to something
> other than -1 by themselves in the past. So do you just mean automatic
> DPI/resolution detection?
yes
> That seems to have been fixed for 38 by
> https://bugzilla.mozilla.org/show_bug.cgi?id=1031073 . Am I missing
> something?
Interesting, I didn't know about that.
Flags: needinfo?(dao)
Comment 15•10 years ago
|
||
Verified fixed on latest Nightly, build ID: 20150602055237 using a MS Pro 2 device running Windows 8.1 64-bit.
Status: RESOLVED → VERIFIED
Assignee | ||
Comment 16•10 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 #8627206 -
Flags: approval-mozilla-beta?
Updated•10 years ago
|
status-firefox40:
--- → affected
Comment 17•10 years ago
|
||
Comment on attachment 8627206 [details] [diff] [review]
Patch for 40
Verified fix in support of Windows 10. Beta+
Attachment #8627206 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 18•10 years ago
|
||
Comment 19•10 years ago
|
||
Also confirming the fix on Firefox 40 beta 2, build ID: 20150706172413.
You need to log in
before you can comment on or make changes to this bug.
Description
•