Closed Bug 1187631 Opened 4 years ago Closed 4 years ago

Reduce caption button height to normal native size

Categories

(Firefox :: Theme, defect)

Unspecified
Windows 10
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 42
Tracking Status
firefox40 --- verified
firefox41 --- fixed
firefox42 --- fixed

People

(Reporter: Dolske, Assigned: Dolske)

Details

Attachments

(2 files)

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.
Attached patch Patch v.1Splinter Review
Assignee: nobody → dolske
Attachment #8638947 - Flags: review?(jaws)
Attached image Screenshots
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.
(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 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+
(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?]
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
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?
https://hg.mozilla.org/mozilla-central/rev/c6a3767c30fc
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
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+
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.