Last Comment Bug 717262 - Test pilot notifications have broken / transparent UI in Thunderbird
: Test pilot notifications have broken / transparent UI in Thunderbird
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: General (show other bugs)
: 13 Branch
: All All
: -- normal (vote)
: Thunderbird 14.0
Assigned To: Mark Banner (:standard8, limited time in Dec)
:
:
Mentors:
https://getsatisfaction.com/mozilla_m...
Depends on:
Blocks: 497995 729127
  Show dependency treegraph
 
Reported: 2012-01-11 09:02 PST by Daniel Holbert [:dholbert]
Modified: 2012-09-07 06:22 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
-
fixed
+
fixed


Attachments
screenshot (210.87 KB, image/png)
2012-01-11 09:02 PST, Daniel Holbert [:dholbert]
no flags Details
The fix (1.41 KB, patch)
2012-03-19 05:10 PDT, Mark Banner (:standard8, limited time in Dec)
no flags Details | Diff | Splinter Review
The fix v2 (1.41 KB, patch)
2012-03-20 06:36 PDT, Mark Banner (:standard8, limited time in Dec)
mconley: review+
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
Details | Diff | Splinter Review
Transparent TestPilot UI on comm-central (43.53 KB, image/png)
2012-03-26 11:57 PDT, Mike Conley (:mconley)
no flags Details

Description Daniel Holbert [:dholbert] 2012-01-11 09:02:27 PST
Created attachment 587722 [details]
screenshot

Today I got a test pilot notification for "A Week in the Life of a Mail Client" over my Thunderbird Daily session.

This popup was partly translucent and near-impossible to read.  Looks pretty horked.  Filing this bug on that.

System info:
 - 32-bit Ubuntu 11.04 (with the default "Unity" desktop).
 - Up-to-date Thunderbird Daily:
    Mozilla/5.0 (X11; Linux i686; rv:12.0a1) Gecko/20120111 Thunderbird/12.0a1
 - Test Pilot for Thunderbird 1.3.4  (bundled I believe; I don't recall explicitly installing it)

(Also, FWIW, I've been running Thunderbird Daily for months, and this is the first Test Pilot notification I've ever gotten there IIRC.)
Comment 1 Mike Conley (:mconley) 2012-03-01 07:00:31 PST
I'm also seeing this on EarlyBird (TB 12).
Comment 2 Mike Conley (:mconley) 2012-03-01 07:04:46 PST
This problem does not seem to occur on beta (TB 11).
Comment 3 Mike Conley (:mconley) 2012-03-01 07:06:19 PST
Steps for diagnosis:

1)  Open up the preferences dialog, and change the mailnews.start_page.override_url to equal chrome://testpilot/content/debug.html
2)  Close the preferences dialog
3)  Go to Help > What's New
4)  In the diagnostic tab that appears, click on "Show Dummy Popup"
Comment 4 Mark Banner (:standard8, limited time in Dec) 2012-03-01 07:07:16 PST
From testing on Mac, I can confirm this isn't happening in TB 11 beta.
Comment 5 Daniel Holbert [:dholbert] 2012-03-01 09:30:48 PST
Confirmed that Comment 3 repro's the problem for me in EarlyBird 12 from yesterday (20120228)
Comment 6 Mike Conley (:mconley) 2012-03-12 06:12:24 PDT
The latest EarlyBird no longer has this issue.

I suspect this is due to the fact that bug 497995 was backed out of mozilla-aurora (which also explains the weird boxes we're currently seeing in the tab selector - being addressed in bug 713852).

So, for the person investigating this - I would investigate how bug 497995 changes the behaviour of border-image.
Comment 7 Mark Banner (:standard8, limited time in Dec) 2012-03-13 05:23:08 PDT
Updating flags per comment 6
Comment 8 Mark Banner (:standard8, limited time in Dec) 2012-03-19 05:10:23 PDT
Created attachment 607123 [details] [diff] [review]
The fix

Well spotted Mike! Here's the fix ported from bug 497995 and attachment 554137 [details] [diff] [review]. I've tested it on Mac on beta, earlybird & trunk and seems fine.

Note: the old lines are left in to support the versions where bug 497995 doesn't apply.
Comment 9 Mike Conley (:mconley) 2012-03-19 07:00:21 PDT
Comment on attachment 607123 [details] [diff] [review]
The fix

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

Hm... I'm a little amazed that this will do the right thing when displayed in older versions of TB...aren't CSS styles applied in order, such that the second -moz-border-image will override the first?

::: mail/app/profile/extensions/tbtestpilot@labs.mozilla.com/content/browser.css
@@ +76,4 @@
>   -moz-border-image: url(chrome://testpilot-os/skin/notification-tail-up.png) 26 56 22 18 / 26px 56px 22px 18px round stretch;
> + // Supported in Gecko >= 13
> + -moz-border-image: url(chrome://testpilot-os/skin/notification-tail-up.png) 26 \
> +50 22 18 fill repeat;

If we're going to break up the long line like this, I'd rather the "50 22 18 fill repeat;" be aligned so that it starts beneath the "u" in "url(chrome://..."

Also, I'm curious - why was that line broken up, but not the one on line 93?
Comment 10 Mark Banner (:standard8, limited time in Dec) 2012-03-20 06:35:45 PDT
(In reply to Mike Conley (:mconley) from comment #9)
> Comment on attachment 607123 [details] [diff] [review]
> The fix
> 
> Review of attachment 607123 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hm... I'm a little amazed that this will do the right thing when displayed
> in older versions of TB...aren't CSS styles applied in order, such that the
> second -moz-border-image will override the first?

When I tested it on an older one, it seemed that the line with "fill" now added was rejected as it didn't conform to spec. The same happened the opposite way around.


> >   -moz-border-image: url(chrome://testpilot-os/skin/notification-tail-up.png) 26 56 22 18 / 26px 56px 22px 18px round stretch;
> > + // Supported in Gecko >= 13
> > + -moz-border-image: url(chrome://testpilot-os/skin/notification-tail-up.png) 26 \
> > +50 22 18 fill repeat;
> 
> If we're going to break up the long line like this, I'd rather the "50 22 18
> fill repeat;" be aligned so that it starts beneath the "u" in
> "url(chrome://..."
> 
> Also, I'm curious - why was that line broken up, but not the one on line 93?

Oops, that was just a copy and paste from emacs in bash terminal error - they weren't intended to be wrapped.
Comment 11 Mark Banner (:standard8, limited time in Dec) 2012-03-20 06:36:29 PDT
Created attachment 607522 [details] [diff] [review]
The fix v2

Correct version per comments.
Comment 12 Mike Conley (:mconley) 2012-03-20 06:38:40 PDT
Comment on attachment 607522 [details] [diff] [review]
The fix v2

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

The fact that it degrades gracefully is rather surprising, but still pleasant. :)

This looks good to me.  Thanks for your work!
Comment 13 Mark Banner (:standard8, limited time in Dec) 2012-03-20 07:08:45 PDT
Checked in:

http://hg.mozilla.org/comm-central/rev/ffb738522343
http://hg.mozilla.org/comm-central/rev/fb3b7c4c409e
Comment 14 Mark Banner (:standard8, limited time in Dec) 2012-03-20 07:11:08 PDT
Comment on attachment 607522 [details] [diff] [review]
The fix v2

[Triage Comment]
Although AMO will provide the latest version, I want to push this out to all branches, so that users installing fresh will get the new version straight off.
Comment 15 Mark Banner (:standard8, limited time in Dec) 2012-03-20 07:35:02 PDT
(In reply to Mark Banner (:standard8) from comment #13)
> Checked in:
> 
> http://hg.mozilla.org/comm-central/rev/ffb738522343
> http://hg.mozilla.org/comm-central/rev/fb3b7c4c409e

Oh the latter of these was for a version bump for test pilot so that we can push this fixed version to AMO.
Comment 17 Mike Conley (:mconley) 2012-03-26 11:54:55 PDT
This appears to be an issue again on comm-central.  Screenshot forthcoming...
Comment 18 Mike Conley (:mconley) 2012-03-26 11:57:29 PDT
Created attachment 609403 [details]
Transparent TestPilot UI on comm-central

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120326 Thunderbird/14.0a1

TestPilot 1.3.8 on Windows 7.  Might affect the other platforms too.
Comment 19 Ludovic Hirlimann [:Usul] 2012-04-17 01:41:22 PDT
Mark any idea why this came back ?
Comment 20 Mark Banner (:standard8, limited time in Dec) 2012-04-17 05:40:36 PDT
I did originally fix it locally, however, it appears that adding the wrong style of comments before I posted the patch broke it again. I thought I had re-tested it before I pushed it to AMO etc, however, somewhere along the line my testing broke down :-(

The good news is that bug 738052 has already fixed this in-tree. So we can get a new version of it out easily.

Note You need to log in before you can comment on or make changes to this bug.