Closed Bug 1260103 Opened 4 years ago Closed 4 years ago

Block plugin pop-up close button is barely visible

Categories

(Firefox :: Theme, defect)

All
Windows
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 48
Tracking Status
firefox45 --- wontfix
firefox46 --- wontfix
firefox47 + verified
firefox48 --- verified

People

(Reporter: cornel_ionce, Assigned: dao)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

[Affected versions]:
- Firefox 45.0.1
- Firefox 46 beta 5
- Latest 47.0a2 Aurora
- Latest 48.0a1 Nightly

[Affected platforms]:
- Windows 10 x64
- Windows 7 x64
- Ubuntu 14.04 x86

[Steps to reproduce]:
1. Make sure you are using an old/blocked flash version. 
2. Open Firefox using a clean profile.
3. Navigate to a flash content website.
4. Notice the outdated plugin prevention message.


[Expected result]:
The message is correctly displayed with every element visible. 

[Actual result]:
The close button is barely visible.

[Regression range]:
I was able to track down a regression range on Windows 10x64.

Last good: 20150717030202
First bad: 20150718030211
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=15155971639c&tochange=d317a93e5161

Caused by: 	
ac6f2fb6777e	Dão Gottwald — Bug 1173729 - Update generic close icon and new tab button icon on Windows 10. r=jaws

[Additional notes]:
- Screenshot: http://i.imgur.com/xx9xP2S.png
- Hovered state: http://i.imgur.com/DBytoRr.png
- This issue does not reproduce on Mac OS X 10.9.5
Dao can you take a look here?
Flags: needinfo?(dao)
OS: All → Windows
This is because we set a custom background for this notification bar: http://mxr.mozilla.org/mozilla-central/search?string=pluginVulnerable&case=on

I'd suggest that we stop doing that and use the standard notification bar styling.
Flags: needinfo?(dao) → needinfo?(benjamin)
We decided explicitly that it was more important for this to be the same styling as the in-content blocked plugin notification. I'd like to keep that if possible. We already have a close icon for this case: are we never using it or only not using it in this new win10 case?
Flags: needinfo?(benjamin)
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #8739365 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8739365 [details] [diff] [review]
patch

Review of attachment 8739365 [details] [diff] [review]:
-----------------------------------------------------------------

This reproduces on Linux as well, at least on Nightly, so we should fix it there, too.
Attachment #8739365 - Flags: review?(gijskruitbosch+bugs)
Attached patch patchSplinter Review
Attachment #8739365 - Attachment is obsolete: true
Attachment #8739376 - Flags: review?(gijskruitbosch+bugs)
Attachment #8739376 - Flags: review?(gijskruitbosch+bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/1ea84d818c7d
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Confirming the fix on Latest Nightly 48, build ID: 20160410030224.
Verified on:
* Windows 10 x64
* Windows 7 x64
* Ubuntu 14.04 x86
Status: RESOLVED → VERIFIED
QA Contact: cornel.ionce
Regression from Firefox 42, wontfix for 46 at this point. 
Ritu, do you want this for beta? css only change, though it is late in beta.
Hi Dao, if this is a low-risk fix, since its a very visible regression I am open to uplifting to Beta47. What do you think?
Flags: needinfo?(rkothari) → needinfo?(dao+bmo)
Comment on attachment 8739376 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1173729
[User impact if declined]: see comment 0
[Describe test coverage new/current, TreeHerder]: no test coverage
[Risks and why]: simple CSS fix, low risk
[String/UUID change made/needed]: none
Flags: needinfo?(dao+bmo)
Attachment #8739376 - Flags: approval-mozilla-aurora?
(In reply to Dão Gottwald [:dao] from comment #12)
> Comment on attachment 8739376 [details] [diff] [review]
> patch
> 
> Approval Request Comment
> [Feature/regressing bug #]: bug 1173729
> [User impact if declined]: see comment 0
> [Describe test coverage new/current, TreeHerder]: no test coverage
> [Risks and why]: simple CSS fix, low risk
> [String/UUID change made/needed]: none

48 is on aurora, and this was fixed in 48 - did you mean to request beta approval?
Flags: needinfo?(dao+bmo)
Flags: needinfo?(dao+bmo)
Attachment #8739376 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?
Comment on attachment 8739376 [details] [diff] [review]
patch

Visual regression, though it has been shipping since 45, the fix is simple and has been verified on Nightly, Beta47+
Attachment #8739376 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Hi Wes, fyi this fix needs to be uplifted to m-b. Thanks!
Flags: needinfo?(wkocher)
Setting the flag for verification on 47.0b9.
Flags: qe-verify+
Flags: needinfo?(cornel.ionce)
Confirming this issue is fixed on Firefox 47.0b9, build ID 20160526140250.
Flags: qe-verify+
Flags: needinfo?(cornel.ionce)
Duplicate of this bug: 1027926
Duplicate of this bug: 1141061
You need to log in before you can comment on or make changes to this bug.