Closed Bug 1188291 Opened 9 years ago Closed 9 years ago

New tab page's notification close button and preferences dialogs' close button look wrong

Categories

(Firefox :: Theme, defect, P1)

Product:
Firefox
Component:
Theme
Version:
40 Branch
Platform:
Unspecified
Windows 10
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 42
Tracking Flags:
Tracking Status
firefox40 --- wontfix
firefox41 --- verified
firefox42 --- verified

People

(Reporter: phlsa, Assigned: dao)

References

Details

(Keywords: regression)

Attachments

(4 files, 2 obsolete files)

newtab.png
(deleted), image/png
Details
prefs-close-icon.PNG
(deleted), image/png
Details
patch v2
(deleted), patch
Sylvestre
: approval-mozilla-aurora+
Sylvestre
: approval-mozilla-beta-
Details | Diff | Splinter Review
aurora patch
(deleted), patch
Details | Diff | Splinter Review
Attached image devedition-issues.png (obsolete) (deleted) —
In dev-editon on Windows 10, the tab close button gets cropped on foreground tabs when hovering (see attachment).
Attached image newtab.png (deleted) —
A similar thing happens on about:newtab when removing a tile (not just in devedition)
The DevEdition issue has been fixed on Nightly, and should be fixed on DevEdition soon as well. Morphing this into the newtab issue. Dao, it seems that your patch regresses the new tab page, we may want to check if it regresses anything else : https://dxr.mozilla.org/mozilla-central/search?q=close-icon&case=false
Flags: needinfo?(dao)
Summary: [dev edition] Close buttons on foreground tabs + hover are cropped → Wrong image region in new tab page's thumbnail notification close button
Attached image prefs-close-icon.PNG (deleted) —
In content prefs have a squashed close icon too.
Blocks: 1173729
No longer blocks: windows-10
Flags: needinfo?(dao)
Keywords: regression
QA Contact: dao
Assignee: nobody → dao
QA Contact: dao
Attached patch patch (obsolete) (deleted) — — Splinter Review
Attachment #8639785 - Attachment is obsolete: true
Attachment #8640448 - Flags: review?(jaws)
Summary: Wrong image region in new tab page's thumbnail notification close button → New tab page's notification close button and preferences dialogs' close button look wrong
Comment on attachment 8640448 [details] [diff] [review] patch >- <caption class="titlebar" flex="1" align="center"> >- <label id="dialogTitle" class="title" flex="1"></label> >+ <caption flex="1" align="center"> >+ <label id="dialogTitle" flex="1"></label> This is drive-by cleanup; the titlebar and title classes are entirely unused as far as I can tell.
Comment on attachment 8640448 [details] [diff] [review] patch Review of attachment 8640448 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!
Attachment #8640448 - Flags: review?(jaws) → review+
Flags: qe-verify+
(In reply to Pulsebot from comment #9) > https://hg.mozilla.org/integration/fx-team/rev/caa59fe73a47 Backed out again. I expected that my other patch for bug 1187219 caused the failure, but apparently it was this one. https://hg.mozilla.org/integration/fx-team/rev/5c66c559c14b
Comment on attachment 8640448 [details] [diff] [review] patch >--- a/toolkit/themes/shared/in-content/common.inc.css >+++ b/toolkit/themes/shared/in-content/common.inc.css >@@ -174,19 +174,18 @@ html|button { > } > > /* xul buttons and menulists */ > > *|button, > xul|colorpicker[type="button"], > xul|menulist { > -moz-appearance: none; >- height: 30px; >+ min-height: 30px; Looks like this change is causing the failure. I'm not going to spend more time investigating this now, I'll just avoid that change.
Attached patch patch v2 (deleted) — — Splinter Review
Attachment #8640448 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/bc7765406851
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Reproduced this issue with 2015-07-27 Nightly. Confirming the fix using latest 42.0a1 Nightly, build ID: 20150802030218.
Status: RESOLVED → VERIFIED
status-firefox42: fixed → verified
QA Contact: cornel.ionce
Dao, does this need uplifting ?
Flags: needinfo?(dao)
(In reply to Tim Nguyen [:ntim] from comment #16) > Dao, does this need uplifting ? Now that it's verified on mozilla-central, yes
Flags: needinfo?(dao)
Comment on attachment 8641514 [details] [diff] [review] patch v2 not sure if Approval Request Comment [Feature/regressing bug #]: bug 1173729 [User impact if declined]: see attachment 8639786 [details] and attachment 8639798 [details] [Describe test coverage new/current, TreeHerder]: n/a [Risks and why]: low [String/UUID change made/needed]: none
Attachment #8641514 - Flags: approval-mozilla-beta?
Attachment #8641514 - Flags: approval-mozilla-aurora?
(In reply to Dão Gottwald [:dao] from comment #18) > not sure if ... we're still able to land patches for Firefox 40 at this point.
It is too late for 40. The gtb of the RC is today and it is not a dev edition...
status-firefox40: --- → wontfix
status-firefox41: --- → affected
Comment on attachment 8641514 [details] [diff] [review] patch v2 Should be low risk, fix a visual regression on the dev edition, taking it.
Attachment #8641514 - Flags: approval-mozilla-beta?
Attachment #8641514 - Flags: approval-mozilla-beta-
Attachment #8641514 - Flags: approval-mozilla-aurora?
Attachment #8641514 - Flags: approval-mozilla-aurora+
(In reply to Sylvestre Ledru [:sylvestre] from comment #20) > It is too late for 40. The gtb of the RC is today and it is not a dev > edition... This bug isn't about dev edition, comment 0 is obsolete.
Needs rebasing for Aurora uplift (or a dep nominated for approval as well).
Flags: needinfo?(dao)
Attached patch aurora patch (deleted) — — Splinter Review
looks like the new tab page issue is only an issue on mozilla-central due to recent changes to that feature
Flags: needinfo?(dao)
(In reply to Dão Gottwald [:dao] from comment #22) > This bug isn't about dev edition, comment 0 is obsolete. OK, my bad. Anyway, this is too late for 40!
Confirming that latest 41.0a2 DevEdition (build ID: 20150804004026) is now fixed on Windows 10 64-bit.
status-firefox41: fixed → verified
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: