1372043 - Icon of minimize button disappeared (but button still works properly)

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Blake]

Attachment: minimize.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0
Build ID: 20170611030208

Steps to reproduce:

The minimize icon goes away when the browser is maximized.

You can either resize to browser to click the maximize button to shrink the window.


Actual results:

The icon goes away


Expected results:

The icon should always be visible

Comment 1

[Alice0775 White]

The minimize button disappears when Maximize -> normal in my case.

Str
1. Start Browser with maximized mode
2. Click normal size mode button
3. Observe the minimized button


Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=35bdd25a810319b52365a12da51df0ac2426c6cc&tochange=806cf3746d111cd359fcea0938959b0582448ad6

Regressed by: 806cf3746d11	UK992 — Bug 1185482 - Remove gap between titlebar buttons and navigation toolbar r=dao

Comment 2

[UK92]

I can't reproduce on 1.25 DPI. What is your DPI?
This could be solved with removing padding top and bottom, and only setting height.

Comment 3

[UK92]

Attachment: Possible solution

Padding top and bottom is removed from titlebar buttons, except when windows is maximized and menu bar visible, because titlebar buttons are more flat.

Comment 4

[UK92]

Attachment: Before and after patch

Comment 5

[UK92]

Attachment: Before and after patch and native controls

Comment 6

[UK92]

Attachment: Bug 1372043 - Set height on titlebar buttons only in miximized window

Review commit: https://reviewboard.mozilla.org/r/149568/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/149568/

Comment 7

[UK92]

Comment on attachment 8878218 [details]
Bug 1372043 - Set height on titlebar buttons only in miximized window

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/149568/diff/1-2/

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 8878218 [details]
Bug 1372043 - Set height on titlebar buttons only in miximized window

https://reviewboard.mozilla.org/r/149568/#review154408

::: browser/base/content/browser-tabsintitlebar.js:179
(Diff revision 2)
> +      isPhotonWindows10 = (AppConstants.MOZ_PHOTON_THEME &&
> +                           AppConstants.isPlatformAndVersionAtLeast("win", "10.0"));
> +
> +      if (isPhotonWindows10) {
> +        // Remove height property from #titlebar-buttonbox to get right #titlebar-content height
> +        $("titlebar-buttonbox").style.removeProperty("height");

Thanks for looking into this. Unfortunately, this breaks the expected order described here:

      // Try to avoid reflows in this code by calculating dimensions first and
      // then later set the properties affecting layout together in a batch.
      
...

      // Begin setting CSS properties which will cause a reflow

Comment 10

[Blake]

This bug looks fixed as of the 2017-06-24 nightly build on my Windows 10 x64 machine.

http://i.imgur.com/Z1a3qFR.png

Comment 11

[Oriol Brufau [:Oriol]]

I can still reproduce with 2017-06-26 nightly:

1. Maximize current window, and make sure the menu bar is hidden
2. Open a new (maximided) window with Ctrl+N
3. Unmaximize the new window, there is this problem.

But if the window is created non-maximized, then it works well.

Comment 15

[Dão Gottwald [:dao]]

Hey UK92, are you still going to try to fix this or should we try to find a new owner?

Comment 16

[UK92]

Comment on attachment 8878218 [details]
Bug 1372043 - Set height on titlebar buttons only in miximized window

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/149568/diff/2-3/

Comment 17

[UK92]

Sorry i forgot about this bug.
Latest patch set height only when window is maximized.

Comment 18

[Dão Gottwald [:dao]]

Comment on attachment 8878218 [details]
Bug 1372043 - Set height on titlebar buttons only in miximized window

https://reviewboard.mozilla.org/r/149568/#review163448

Thanks! I can't seem to reproduce the bug with or without this patch, but at least it shouldn't make things worse.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/62a148ad6d3b
Set height on titlebar buttons only in miximized window r=dao

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/62a148ad6d3b

Comment 21

[Madhuri]

[bugday-20170726] 

status-ff56.0a1 : FIXED & VERIFIED.

Managed to reproduce the issue on Firefox Nightly under Windows 10 X 64.

The issue is no longer reproducible on Firefox latest Nightly [BuildID : 20170725030209 , 56.0a1(2017-07-25)(32 bit)]. 
Tests were performed under Windows 10 X 64.

Comment 22

[Deac Alin]

Verified on Windows 10 x 64 and 32 bit on latest Nightly 57.0a1 Build ID 20170815100349 and i cannot reproduce the issue. I will mark this as verified fixed. Thank you