717262 - Test pilot notifications have broken / transparent UI in Thunderbird

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Daniel Holbert [:dholbert]]

Attachment: 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) (:⚙️)]

I'm also seeing this on EarlyBird (TB 12).

Comment 2

[Mike Conley (:mconley) (:⚙️)]

This problem does not seem to occur on beta (TB 11).

Comment 3

[Mike Conley (:mconley) (:⚙️)]

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)]

From testing on Mac, I can confirm this isn't happening in TB 11 beta.

Comment 5

[Daniel Holbert [:dholbert]]

Confirmed that Comment 3 repro's the problem for me in EarlyBird 12 from yesterday (20120228)

Comment 6

[Mike Conley (:mconley) (:⚙️)]

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)]

Updating flags per comment 6

Comment 8

[Mark Banner (:standard8)]

Attachment: 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) (:⚙️)]

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/[e-mail]/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)]

(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)]

Attachment: The fix v2

Correct version per comments.

Comment 12

[Mike Conley (:mconley) (:⚙️)]

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)]

Checked in:

http://hg.mozilla.org/comm-central/rev/ffb738522343
http://hg.mozilla.org/comm-central/rev/fb3b7c4c409e

Comment 14

[Mark Banner (:standard8)]

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)]

(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 16

[Mark Banner (:standard8)]

Checked in:

http://hg.mozilla.org/releases/comm-aurora/rev/394d81db3262
http://hg.mozilla.org/releases/comm-aurora/rev/a42983902c5d
http://hg.mozilla.org/releases/comm-beta/rev/45159577008d
http://hg.mozilla.org/releases/comm-beta/rev/5c92b414af33

Comment 17

[Mike Conley (:mconley) (:⚙️)]

This appears to be an issue again on comm-central.  Screenshot forthcoming...

Comment 18

[Mike Conley (:mconley) (:⚙️)]

Attachment: 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]]

Mark any idea why this came back ?

Comment 20

[Mark Banner (:standard8)]

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.