352456 - notification flashes before sliding in

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Michael Wu [:mwu]]

Because we need to calculate the height of the notification before we can setup the slide animation, this can cause the notification to be briefly visible before sliding in.

Comment 1

[Robert Strong (they/them - no direct email)]

Also, the content below the notificationbox immediately shifts down when the height is calculated, shifts back up, and then shift gradually down as the box gradually increases in height. Perhaps it is possible to
1) get the height by floating over the content while invisible
2) have a containing box around the current notificationbox with the current notificationbox's content invisible to get the height, set the height on the containing box so the main content doesn't shift again, and then animate the current notificationbox to full height.

Comment 2

[Michael Wu [:mwu]]

Attachment: WIP: Hide the height calculation

Messy, but it seems to work.

Comment 3

[Robert Strong (they/them - no direct email)]

Attachment: simple patch that works for me

Neal and Michael, this wfm in the content window as well as in the add-ons mgr with the patch from Bug 348576 using multiple notifications... is there any reason not to go with this approach? Would this be too much for 1.8.1?

Comment 4

[Michael Wu [:mwu]]

(In reply to comment #3)
> Created an attachment (id=238182) [edit]
> simple patch that works for me
> 
> Neal and Michael, this wfm in the content window as well as in the add-ons mgr
> with the patch from Bug 348576 using multiple notifications... is there any
> reason not to go with this approach? Would this be too much for 1.8.1?
> 
This does not fix the problem - it merely hides it well enough for your system. To see what I mean, put an alert() at the very beginning of _showNotification. Also, notifications in popup windows w/o any browser UI will continue to flash, AFAICT.

Comment 5

[Neil Deakin]

Is a special testcase needed to see this issue?

Comment 6

[Robert Strong (they/them - no direct email)]

It doesn't flash in the Add-ons mgr. at all with this patch on Win32 and I verified that without this patch it doesn't flash on Mac OS X which is probably why this was never reported. Why shouldn't setting these style properties fix this?

Comment 7

[Robert Strong (they/them - no direct email)]

(In reply to comment #5)
> Is a special testcase needed to see this issue?
No, it is readily apparent on Win32 with normal notifications.

Comment 8

[Michael Wu [:mwu]]

Both of these patches also have an issue with calculating the height when there is already a visible notification.

Comment 9

[Michael Wu [:mwu]]

Attachment: Hide height calculation, v2

This should always calculate the correct height without flashing the notification.

Comment 10

[Robert Strong (they/them - no direct email)]

Thanks for doing this Michael!

Comment 11

[Neil Deakin]

Comment on attachment 238258 [details] [diff] [review]
Hide height calculation, v2

Seems OK

Comment 12

[Michael Wu [:mwu]]

Checking in toolkit/content/widgets/notification.xml;
/cvsroot/mozilla/toolkit/content/widgets/notification.xml,v  <--  notification.xml
new revision: 1.10; previous revision: 1.9
done

Comment 13

[Robert Strong (they/them - no direct email)]

Comment on attachment 238258 [details] [diff] [review]
Hide height calculation, v2

I'd like this on branch to fix the visual glitch seen on Win32.. also so Bug 348576 could land on the branch since the visual glitch is even more pronounced in the add-ons mgr. when using notificationbox.

Comment 14

[Mike Connor [:mconnor]]

Comment on attachment 238258 [details] [diff] [review]
Hide height calculation, v2

a=mconnor on behalf of drivers for 1.8 branch checkin

Comment 15

[Michael Wu [:mwu]]

Patch applies with -F 3.

Branch:
Checking in toolkit/content/widgets/notification.xml;
/cvsroot/mozilla/toolkit/content/widgets/notification.xml,v  <--  notification.xml
new revision: 1.1.2.8; previous revision: 1.1.2.7
done