1557102 - Inline styles initially ignored in overlays

Status: RESOLVED FIXED

(Thunderbird :: Add-Ons: General, defect)

Description

[rsjtdrjgfuzkfg (Dirk Steinmetz)]

Attachment: Add-on reproducing the issue

Inline XUL styles are not evaluated when overlays load. They seem, however, to be properly evaluated if something else triggers re-evaluation later on.

STR:
0. Last Thunderbird68 nightly, clean profile. (tested on Linux x64, might affect other platforms as well)

1. Install the attached add-on

Expected:
The main window should not change, as the image in the overlay is styled as display:none.

Actual:
A (stretched) warning sign image appears, with the default width of the warning sign image and transparent background. When inspecting the image in the developer toolbox, it immediately disappears.

About the reproduction add-on:
The add-on simply adds an image element with the alert-icon class and the style
display:none;width:500px;background-color:red to the tabmail-container hbox. All inline styles are not evaluated immediately, while the CSS rules for alert-icon work fine.

Comment 1

[Geoff Lankow (:darktrojan)]

That's interesting, but I doubt anything can be done about it.

Comment 2

[rsjtdrjgfuzkfg (Dirk Steinmetz)]

Attachment: possible workaround

If we can't fix the actual issue, can we at least land a workaround? I attached a proposal. Imho it would be better to have a central workaround in Thunderbird than having individual workarounds in all add-ons.

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: bad calendar colours.png

Comment 4

[Jorg K (CEST = GMT+2)]

Attachment: good calendar colours.png

Well, in TB 68 beta I sometimes see a "bleached" calendar. Should I file a new bug for this?

Comment 5

[Geoff Lankow (:darktrojan)]

Jorg, I haven't seen that in the calendar views themselves, but it looks a lot like bug 1558384 to me.

Comment 6

[Geoff Lankow (:darktrojan)]

Comment on attachment 9072198 [details] [diff] [review]
possible workaround

I'm somewhat surprised that node has a style property at this point, but apparently it does, so let's use it.

[Side note: it would be helpful if your patches were in git format, as our review tools like it better. Example setup here, if you need it: https://developer.mozilla.org/en-US/docs/Mozilla/Mercurial/Installing_Mercurial#Basic_configuration]

Comment 7

[rsjtdrjgfuzkfg (Dirk Steinmetz)]

Great! The workaround patch was created on comm-beta, but applies cleanly on comm-central as well; thus requesting checkin for both.

[Sorry about the commit formatting, missed that when cloning the repo. I'll try to do better next time :) ]

Comment 8

[rsjtdrjgfuzkfg (Dirk Steinmetz)]

Comment on attachment 9072198 [details] [diff] [review]
possible workaround

[Approval Request Comment]
User impact if declined: some add-ons don't work as expected, to fix them they'd need to ship individual workarounds
Testing completed (on c-c, etc.): tested on comm-beta
Risk to taking this patch (and alternatives if risky): doesn't seem risky to me, the change only affects overlays that were already broken without the patch and the added xpath query did not cause any noticeable performance impact on my machine

Comment 9

[Jorg K (CEST = GMT+2)]

(In reply to Geoff Lankow (:darktrojan) from comment #6)

[Side note: it would be helpful if your patches were in git format, as our
review tools like it better. ...]

Umm, what is the issue? I imported the patch without problem for landing, and the header was correct, too. OK, diff -r 242c6d081cdb -r 58a5bc10be95 common/src/Overlays.jsm is not what we use, but which negative effect does it have?

Comment 10

[Geoff Lankow (:darktrojan)]

Bugzilla's diff view thinks the file being changed is called 242c6d081cdb. Not really a big deal but annoying nonetheless.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/686ff7f6f966
Work around inline styles being ignored. r=darktrojan

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/00c23c4b01df
Follow-up: fix linting issue. r=darktrojan (via IRC) DONTBUILD

Comment 13

[Jorg K (CEST = GMT+2)]

TB 68 beta 2:
https://hg.mozilla.org/releases/comm-beta/rev/ac5ddd1d4a94f84856615ed5904fa6d70da4bcbb
https://hg.mozilla.org/releases/comm-beta/rev/11375fbab40b42d74ad33aef4e651690f2b03c79