Closed
Bug 1187631
Opened 9 years ago
Closed 9 years ago
Reduce caption button height to normal native size
Categories
(Firefox :: Theme, defect)
Tracking
()
RESOLVED
FIXED
Firefox 42
People
(Reporter: Dolske, Assigned: Dolske)
Details
Attachments
(2 files)
856 bytes,
patch
|
jaws
:
review+
lmandel
:
approval-mozilla-aurora+
lmandel
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
46.90 KB,
image/png
|
Details |
Spinning this out from bug 1186250. The initial design was to have the window caption button fill the entire height from the navbar to the top of the window (stylistically like Edge). However due to the drag/resize space complications noted in 1186250, that isn't something we can accomplish for Firefox 40. So, per comments 5-7 there, we should reduce the size of our current caption buttons so they match the normal native size.
Assignee | ||
Comment 1•9 years ago
|
||
Assignee: nobody → dolske
Attachment #8638947 -
Flags: review?(jaws)
Assignee | ||
Comment 2•9 years ago
|
||
I took a number of screenshots at different scaling factors, comparing Firefox (left) to the Win10 settings app (middle) and Edge (right). This is on top of the current patch in bug 1184934. We're 1 (physical) pixel short of the native size at 150%, but the correct size everywhere else. Given that there are a number of other subtle off-by-one size/position nits with our caption glyphs (due to the rounding issues with scaling, I presume), this seems fine to me. Not noticeable, and a big improvement over where we currently are.
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Justin Dolske [:Dolske] from comment #2) > We're 1 (physical) pixel short of the native size at 150% Oops, I meant at 125%.
Comment 4•9 years ago
|
||
Comment on attachment 8638947 [details] [diff] [review] Patch v.1 Review of attachment 8638947 [details] [diff] [review]: ----------------------------------------------------------------- This actually reduces it by 4px instead of 2px. Is that right? Nonetheless, the screenshots look fine. rs=me
Attachment #8638947 -
Flags: review?(jaws) → review+
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #4) > This actually reduces it by 4px instead of 2px. Is that right? Yep. The original spec (attachment 8636847 [details]) had them be 36px tall (2 * 12px margin + 12 px glyph box), which is what was implemented. I'm chopping off 4px to make them 32px tall, which matches the measured size of native apps (see screenshot) and is coincidentally what shorlander says in bug 1186250 comment 7. :) [Maybe you're thinking of bug 1173731, which is about removing 2px from a different thing?]
Assignee | ||
Comment 6•9 years ago
|
||
url: https://hg.mozilla.org/integration/fx-team/rev/c6a3767c30fc845c43988b1870ed343430fe40b3 changeset: c6a3767c30fc845c43988b1870ed343430fe40b3 user: Justin Dolske <dolske@mozilla.com> date: Sun Jul 26 12:06:21 2015 -0700 description: Bug 1187631 - Reduce caption button height to normal native size. r=jaws
Assignee | ||
Comment 7•9 years ago
|
||
Comment on attachment 8638947 [details] [diff] [review] Patch v.1 Approval Request Comment [Feature/regressing bug #]: Windows 10 [User impact if declined]: Caption button are unnaturally tall, since we're implementing these ourselves it's important to make them look natural to avoid "Firefox is weird because XUL" type impressions. [Describe test coverage new/current, TreeHerder]: n/a [Risks and why]: Low risk, simple and well understood - only affects Windows 10, and has been tested at various scaling levels. [String/UUID change made/needed]: n/a
Attachment #8638947 -
Flags: approval-mozilla-beta?
Attachment #8638947 -
Flags: approval-mozilla-aurora?
Comment 8•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c6a3767c30fc
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Comment 9•9 years ago
|
||
Comment on attachment 8638947 [details] [diff] [review] Patch v.1 Although this fix just landed on m-c, it is a very low risk fix with the only potential impact being to Windows 10, where the functionality is already incorrect. Beta+ Aurora+
Attachment #8638947 -
Flags: approval-mozilla-beta?
Attachment #8638947 -
Flags: approval-mozilla-beta+
Attachment #8638947 -
Flags: approval-mozilla-aurora?
Attachment #8638947 -
Flags: approval-mozilla-aurora+
Comment 10•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/f61fc9ae115f
status-firefox41:
--- → fixed
Comment 11•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/ead679254aa6
status-firefox40:
--- → fixed
Comment 12•9 years ago
|
||
Reproduced the bug with Nightly 42.0a1 (2015-07-27) - http://i.imgur.com/iAz1NBm. This is verified fixed on Beta 40.0b8 (20150727174134), using Windows 10 Pro x64 (Build 10240). See http://i.imgur.com/8XXCgkk.png.
You need to log in
before you can comment on or make changes to this bug.
Description
•