Closed Bug 1151412 Opened 5 years ago Closed 5 years ago

Question mark icons in conversation window

Categories

(Firefox :: Theme, defect, P1)

defect
Points:
3

Tracking

()

VERIFIED FIXED
Firefox 42
Iteration:
42.2 - Jul 27
Tracking Status
firefox40 + verified
firefox41 --- verified
firefox42 --- verified

People

(Reporter: bogdan_maris, Assigned: dao)

References

Details

(Keywords: regression)

Attachments

(3 files, 1 obsolete file)

Affected builds:
- latest Nightly

Affected OS`s:
- Ubuntu 14.04 (Linux only)

STR:
1. Start Firefox
2. Click the Hello icon
3. Start a Conversation

Expected results: All icons are properly displayed.

Actual results: Icons from conversation window are replaced with question mark icons.

Notes:
- This is a regression:

m-c tinderbox builds:

Last good build: https://hg.mozilla.org/mozilla-central/rev/035959eef3f9
First bad build: https://hg.mozilla.org/mozilla-central/rev/eeb9438975a5
[Tracking Requested - why for this release]:
I'm told that Firefox 40 Beta is affected by this, sounds like something we should track.
Hope we get a fix in time for FF40 release. The fact that the icons are not displayed correctly may indicate that this feature is broken on this version of linux.
I get the same issue on Mac (Yosemite) using Nightly.
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Iteration: --- → 42.2 - Jul 27
Points: --- → 3
Flags: qe-verify+
Flags: firefox-backlog+
Priority: -- → P1
Summary: [Linux] Question mark icons in conversation window → Question mark icons in conversation window
This will fix it for OSX and Linux, which both had rules that were defined lower in the browser.css file, thus taking precedence merely because of their line placement.
It was quite difficult for me to find a regression range, because MattN did a nice cleanup and jaws was the last to touch this file at this place, but I don't see that causing this issue.

Jared, are you comfortable reviewing this?
Attachment #8635233 - Flags: review?(jaws)
However, it just *might* have been bug 1023604...
(In reply to Mike de Boer [:mikedeboer] from comment #5)
> However, it just *might* have been bug 1023604...

This bug was filed 3 months ago but the bug 1023604 is pretty recent.
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6)
> This bug was filed 3 months ago but the bug 1023604 is pretty recent.

OK, well, my research wasn't particularly comprehensive! I was already pretty sure that it was something else that caused this regression.
Are you still OK with reviewing this?
(In reply to Mike de Boer [:mikedeboer] from comment #7)
> (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6)
> > This bug was filed 3 months ago but the bug 1023604 is pretty recent.
> 
> OK, well, my research wasn't particularly comprehensive! I was already
> pretty sure that it was something else that caused this regression.
> Are you still OK with reviewing this?

Yes, I am in the process of doing a clobber on my Linux VM. Can you hop on IRC? I have some questions about why this is only a bug on Linux but this patch makes changes for all platforms. I have a feeling a different selector either needs to be made less specific or something else.
You can use the devtools to see what selector is being used that has higher specificity than the one that you're adjusting.
This is coming from:

.default-notification-icon,
#default-notification-icon {
  list-style-image: url(moz-icon://stock/gtk-dialog-info?size=16);
}

in /browser/themes/linux/browser.css
Blocks: 1150006
Flags: needinfo?(dao)
Keywords: regression
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #8)
> Yes, I am in the process of doing a clobber on my Linux VM. Can you hop on
> IRC? I have some questions about why this is only a bug on Linux but this
> patch makes changes for all platforms. I have a feeling a different selector
> either needs to be made less specific or something else.

Oh, no... this is also occurring on OSX! I updated the summary, but forgot to update the platform field :(

So this is fixing things for both OSs.
OS: Linux → All
Attached patch patchSplinter Review
Does this help? This patch also completely removes the default-notification-icon class. I'm not sure if that class is good for anything that I'm missing.
Assignee: mdeboer → dao
Attachment #8635233 - Attachment is obsolete: true
Flags: needinfo?(dao)
Attachment #8635678 - Flags: review?(mdeboer)
Component: Client → Theme
Product: Hello (Loop) → Firefox
(In reply to Mike de Boer [:mikedeboer] from comment #11)
> Oh, no... this is also occurring on OSX! I updated the summary, but forgot
> to update the platform field :(

TBH I didn't notice the summary was made more generic, I had just read comment #0 and stopped there. :)
Comment on attachment 8635678 [details] [diff] [review]
patch

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

Much better :) Thanks!

I'm sure I don't need to tell you, but don't forget to push this with metadata attached ;-)
Somehow that was stripped from this patch.
Attachment #8635678 - Flags: review?(mdeboer) → review+
https://hg.mozilla.org/mozilla-central/rev/81d867f56456
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Dao, can we have the uplift request to aurora and beta ? Thanks
Flags: needinfo?(dao)
Comment on attachment 8635678 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1150006 on Linux, older regression on OS X
[User impact if declined]: see comment 0
[Describe test coverage new/current, TreeHerder]: n/a
[Risks and why]: fairly low
[String/UUID change made/needed]: none
Flags: needinfo?(dao)
Attachment #8635678 - Flags: approval-mozilla-beta?
Attachment #8635678 - Flags: approval-mozilla-aurora?
Comment on attachment 8635678 [details] [diff] [review]
patch

Approving for uplift to Aurora. Seems like a safe and simple fix.
Attachment #8635678 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
QA Contact: bogdan.maris
Florin, could you please verify this fix on nightly? Just want to make sure it's verified before we uplift to Beta.
Flags: needinfo?(florin.mezei)
Verified that this is fixed on latest Nightly and tinderbox Aurora under Ubuntu 14.04 32-bit and Mac OS X 10.10.4. Marking the bug as verified fixed because the fix reached its target milestone.
Status: RESOLVED → VERIFIED
Flags: needinfo?(florin.mezei)
Comment on attachment 8635678 [details] [diff] [review]
patch

Let's get this verified fix into beta7. Beta+
Attachment #8635678 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Also verified as fixed with Firefox 40 beta 7 across platforms (Windows 7 64-bit, Mac OS X 10.10.4, Windows 10 32-bit and Ubuntu 14.04 32-bit).
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.