350157 - Notification bar doesn't open fully

Status: RESOLVED DUPLICATE of bug 358512

(Toolkit :: UI Widgets, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

Usually notification bar doesn't open fully. It seems like the animation
ends too early, missing perhaps one or two steps.

Perhaps a dup, but couldn't find ...

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1b2) Gecko/20060823 BonEcho/2.0b2

Comment 1

[Adam Guthrie]

Do you mean the one that opens when a popup is blocked, for example?

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

yes

Comment 3

[moz_bug_r_a4]

http://lxr.mozilla.org/mozilla/source/toolkit/content/widgets/notification.xml#210

Since |margin| is a floating-point number, in the 4th step of the animation,
|margin + change >= 0| can be false, and |(margin + change) + "px"| can be
turned into a string using exponential notation.  When it happens, since a
number in exponential notation is invalid for CSS properties, style.marginTop
property stays unchanged, and an interval timer continues running -- you can
see continuous CSS error warnings in JS console if you have set
javascript.options.showInConsole pref to true.

I can reproduce this on Windows but cannot on Linux.  The behaviors depend on
theme.

-----
On Windows with the default theme

notification with button:
height = 28,  change = 7
       margin              margin + change
1 : -28.000001907348633  -21.000001907348633
2 : -21.000001907348633  -14.000001907348633
3 : -14.000000953674316  -7.000000953674316
4 : -7.000000476837158   -4.76837158203125e-7  <--
5 : -7.000000476837158   -4.76837158203125e-7
6 : -7.000000476837158   -4.76837158203125e-7
...

notification without button:
height = 26,  change = 6.5
       margin              margin + change
1 : -26.000001907348633  -19.500001907348633
2 : -19.533334732055664  -13.033334732055664
3 : -13.066667556762695  -6.566667556762695
4 : -6.600000381469727   -0.10000038146972656
5 : -0.13333334028720856  6.366666659712791

-----
On Linux with the default theme

notification with button:
height = 34,  change = 8.5
       margin              margin + change
1 : -34                  -25.5
2 : -25.533334732055664  -17.033334732055664
3 : -17.066667556762695  -8.566667556762695
4 : -8.600000381469727   -0.10000038146972656
5 : -0.13333334028720856  8.366666659712791

notification without button:
height = 26,  change = 6.5
       margin              margin + change
1 : -26.000001907348633  -19.500001907348633
2 : -19.533334732055664  -13.033334732055664
3 : -13.066667556762695  -6.566667556762695
4 : -6.600000381469727   -0.10000038146972656
5 : -0.13333334028720856  6.366666659712791

-----

This could be fixed by using Number.prototype.toFixed method.  But, I think
that it doesn't make sense to set style.marginTop to a value that is too close
to 0.

How about this:
- if (change > 0 && margin + change >= 0) {
+ if (change > 0 && margin + change > -1) {
    aNotification.style.marginTop = "0px";
    aNotification.style.opacity = 1;
    done = true;
  }

Comment 4

[moz_bug_r_a4]

Attachment: testcase

This uses <style>notification { height: 40px; }</style> to expose the bug.

height = 40,  change = 10
       margin              margin + change
1 : -40.000003814697266  -30.000003814697266
2 : -30.000001907348633  -20.000001907348633
3 : -20.000001907348633  -10.000001907348633
4 : -10.000000953674316  -9.5367431640625e-7
5 : -10.000000953674316  -9.5367431640625e-7
6 : -10.000000953674316  -9.5367431640625e-7
...

Comment 5

[moz_bug_r_a4]

Attachment: Screenshot

Comment 6

[moz_bug_r_a4]

Attachment: patch

- if (change > 0 && margin + change >= 0) {
+ if (change > 0 && margin + change > -1) {

Comment 7

[Adam Guthrie]

Ugh, looks like Bugzilla cleared the review flag when I changed components. Please re-request, [e-mail].

Comment 8

[:Gavin Sharp [email: [e-mail]]]

*** Bug 358512 has been marked as a duplicate of this bug. ***

Comment 9

[Neil Deakin]

Comment on attachment 243341 [details] [diff] [review]
patch

I think I like the patch in bug 358512 more.

Comment 10

[moz_bug_r_a4]

Comment on attachment 243341 [details] [diff] [review]
patch

Ok. I have no reason to persist in this patch :)

Comment 11

[:Gavin Sharp [email: [e-mail]]]

*** This bug has been marked as a duplicate of 358512 ***