752216 - Port |Bug 641892 - Support showing multiple popup notification icons at the same time| to SeaMonkey

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect, P2)

Description

[Serge Gautherie (:sgautherie)]

{
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/suite/browser/test/browser_popupNotification.js | main action #1 was clicked
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/suite/browser/test/browser_popupNotification.js | only one notification displayed - Got 3, expected 1
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/suite/browser/test/browser_popupNotification.js | only one notification displayed - Got 2, expected 1
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/suite/browser/test/browser_popupNotification.js | message matches - Got This is popup notification test-notification-7 from test 7 1, expected This is popup notification test-notification-6 from test 6
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/suite/browser/test/browser_popupNotification.js | id matches - Got test-notification-7-notification, expected test-notification-6-notification
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/suite/browser/test/browser_popupNotification.js | Test timed out

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_dragdrop.js | an unexpected uncaught JS exception reported through window.onerror - ReferenceError: info is not defined at chrome://mochitests/content/browser/suite/browser/test/browser_popupNotification.js:96
[And a lot more of these: bustage after timeout.]
}

*****

Neil (or anyone else), could you do this one?
(Wrt css...)

Comment 1

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Attachment: (Av1) Update notification-anchor-icon rules in navigator.css [Checked in: Comment 4]

I seem that failing tests is caused by no reflected CSS changed in Bug 641892.
This patch may pass tests but I have not adjusted styles (in Bug 641892, I adjusted the margin of anchor icons).

Comment 2

[Serge Gautherie (:sgautherie)]

Comment on attachment 621296 [details] [diff] [review]
(Av1) Update notification-anchor-icon rules in navigator.css
[Checked in: Comment 4]

I confirm this minimal port fixes the existing test ;-)

I'll update the test in a separate patch.
Can you check/deal with the theme updates?

Comment 3

[[e-mail]]

Comment on attachment 621296 [details] [diff] [review]
(Av1) Update notification-anchor-icon rules in navigator.css
[Checked in: Comment 4]

I'm not actually sure that I like Firefox's theme changes, so don't bother.

Comment 4

[Serge Gautherie (:sgautherie)]

Comment on attachment 621296 [details] [diff] [review]
(Av1) Update notification-anchor-icon rules in navigator.css
[Checked in: Comment 4]

http://hg.mozilla.org/comm-central/rev/1b5307fe831b

***

See
https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
for future patches ;-)

Comment 5

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1-FF) browser_popupNotification.js: Fix nits [Checked in: Comment 15]

Back-port from SeaMonkey.

Comment 6

[Serge Gautherie (:sgautherie)]

Attachment: (Cv1) browser_popupNotification.js: Port |Bug 617553 - Doorhanger for add-on installation failures inside the AOM has no parent with chrome hidden|

The feature was ported in bug 595810 part 10.
The test was explicitly not included in bug 595810 test update.
I am simply adding it with a s/is/todo_is/ and a comment, fwiw.

Comment 7

[Serge Gautherie (:sgautherie)]

Attachment: (Dv1) Port |Bug 587587 - Always dismiss popup notifications when they are hidden, and add "removeOnDismissal" option to show()| [Checked in: See comments 24, 31, 35]

Trivial copy.

Comment 8

[Serge Gautherie (:sgautherie)]

Attachment: (Ev1) Port |Bug 670851 - browser_bug519216.js, browser_popupNotification.js and browser_library_middleclick.js cause exception in tabbrowser.xml| [Checked in: Comments 26, 32, 36]

Trivial copy.

Comment 9

[Serge Gautherie (:sgautherie)]

Attachment: (Fv1) browser_popupNotification.js: Port |Bug 641892 - Support showing multiple popup notification icons at the same time| [Checked in: Comment 27]

Trivial copy (with 2 ',' nits).

Comment 10

[[e-mail]]

Comment on attachment 621411 [details] [diff] [review]
(Cv1) browser_popupNotification.js: Port |Bug 617553 - Doorhanger for add-on installation failures inside the AOM has no parent with chrome hidden|

So you didn't want to try to simulate chromeless mode within the test?

>+      loadURI("about:addons", function() {
Is there any point in loading about:addons?

Comment 11

[[e-mail]]

Comment on attachment 621412 [details] [diff] [review]
(Dv1) Port |Bug 587587 - Always dismiss popup notifications when they are hidden, and add "removeOnDismissal" option to show()|
[Checked in: See comments 24, 31, 35]

>   // Test notification when chrome is hidden
>-  { // Test #18
>+  { // Test #19
???

Comment 12

[Serge Gautherie (:sgautherie)]

(In reply to [e-mail] from comment #10)

> So you didn't want to try to simulate chromeless mode within the test?

I read you suggested that, but my point wrt this bug is only to resync' the test.

If you want that, I suggest you file a follow-up bug to add a "SeaMonkey 19bis" test, or to rewrite the "Firefox 19" test in a way that would work on both applications.
(At least, there will be a todo ftb :-|)
In this case, I thought we should either fix the application or let the test be.

> >+      loadURI("about:addons", function() {
> Is there any point in loading about:addons?

I don't know: the test pass and is documented. That's all I care about ftb.


(In reply to [e-mail] from comment #11)
> >-  { // Test #18
> >+  { // Test #19
> ???

Patch Cv1 adds a second "18", which is renamed in patch Dv1:
I preferred to do these ports exactly as they happened in Firefox.

Comment 13

[[e-mail]]

(In reply to Serge Gautherie from comment #12)
> (In reply to comment #10)
> > So you didn't want to try to simulate chromeless mode within the test?
> I read you suggested that, but my point wrt this bug is only to resync' the
> test.
> 
> If you want that, I suggest you file a follow-up bug to add a "SeaMonkey
> 19bis" test, or to rewrite the "Firefox 19" test in a way that would work on
> both applications.
This works on SeaMonkey, but I have no idea whether it will work on Firefox:
  { // Test #19
    run: function () {
      window.locationbar.visible = false;
      this.notifyObj = new basicNotification();
      this.notification = showNotification(this.notifyObj);
      window.locationbar.visible = true;
    },

> (In reply to comment #11)
> > >-  { // Test #18
> > >+  { // Test #19
> > ???
> Patch Cv1 adds a second "18", which is renamed in patch Dv1:
> I preferred to do these ports exactly as they happened in Firefox.
Well I really don't see the point of that, I want the 19th test to be numbered 19 when it lands please!

Comment 14

[[e-mail]]

Comment on attachment 621412 [details] [diff] [review]
(Dv1) Port |Bug 587587 - Always dismiss popup notifications when they are hidden, and add "removeOnDismissal" option to show()|
[Checked in: See comments 24, 31, 35]

r=me except as per comment #11.

Comment 15

[Serge Gautherie (:sgautherie)]

Comment on attachment 621410 [details] [diff] [review]
(Bv1-FF) browser_popupNotification.js: Fix nits
[Checked in: Comment 15]

https://hg.mozilla.org/mozilla-central/rev/835d5fab89dc

Comment 16

[Serge Gautherie (:sgautherie)]

Attachment: (Gv1-FF) browser_popupNotification.js: Use locationbar to simplify test 19 [Checked in: Comment 21]

Per comment 13.

Comment 17

[[e-mail]]

Comment on attachment 621578 [details] [diff] [review]
(Gv1-FF) browser_popupNotification.js: Use locationbar to simplify test 19
[Checked in: Comment 21]

Not quite sure what the point is since all the new code is mine anyway...

Comment 18

[Serge Gautherie (:sgautherie)]

Comment on attachment 621578 [details] [diff] [review]
(Gv1-FF) browser_popupNotification.js: Use locationbar to simplify test 19
[Checked in: Comment 21]

Succeeded as
https://tbpl.mozilla.org/?tree=Try&rev=8a9296e48cdc

Comment 19

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

Does the test still fail if you break the relevant functionality?

Comment 20

[Serge Gautherie (:sgautherie)]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #19)
> Does the test still fail if you break the relevant functionality?

I tested that the check fails if I comment out the "visible = false" line.

Comment 21

[Serge Gautherie (:sgautherie)]

Comment on attachment 621578 [details] [diff] [review]
(Gv1-FF) browser_popupNotification.js: Use locationbar to simplify test 19
[Checked in: Comment 21]

https://hg.mozilla.org/mozilla-central/rev/cb6759edb577

Comment 22

[Serge Gautherie (:sgautherie)]

Attachment: (Cv2) browser_popupNotification.js: Port |Bug 617553 - Doorhanger for add-on installation failures inside the AOM has no parent with chrome hidden|, Port patch Gv1-FF [Checked in: See comments 23+28, 30, 34(*2)]

Cv1, with comment 13 suggestion(s).

Comment 23

[Serge Gautherie (:sgautherie)]

Comment on attachment 622585 [details] [diff] [review]
(Cv2) browser_popupNotification.js: Port |Bug 617553 - Doorhanger for add-on installation failures inside the AOM has no parent with chrome hidden|, Port patch Gv1-FF
[Checked in: See comments 23+28, 30, 34(*2)]

http://hg.mozilla.org/comm-central/rev/89683cf236ec


[Approval Request Comment]
No risk, test-only.

Comment 24

[Serge Gautherie (:sgautherie)]

Comment on attachment 621412 [details] [diff] [review]
(Dv1) Port |Bug 587587 - Always dismiss popup notifications when they are hidden, and add "removeOnDismissal" option to show()|
[Checked in: See comments 24, 31, 35]

http://hg.mozilla.org/comm-central/rev/fea194b18caa


[Approval Request Comment]
No risk, test-only.

Comment 25

[Serge Gautherie (:sgautherie)]

Comment on attachment 621412 [details] [diff] [review]
(Dv1) Port |Bug 587587 - Always dismiss popup notifications when they are hidden, and add "removeOnDismissal" option to show()|
[Checked in: See comments 24, 31, 35]

(In reply to Serge Gautherie (:sgautherie) from comment #24)
> http://hg.mozilla.org/comm-central/rev/fea194b18caa

Dv1, with comment 13 suggestion(s).

Comment 26

[Serge Gautherie (:sgautherie)]

Comment on attachment 621413 [details] [diff] [review]
(Ev1) Port |Bug 670851 - browser_bug519216.js, browser_popupNotification.js and browser_library_middleclick.js cause exception in tabbrowser.xml|
[Checked in: Comments 26, 32, 36]

http://hg.mozilla.org/comm-central/rev/5fdf33810460

[Approval Request Comment]
No risk, test-only.

Comment 27

[Serge Gautherie (:sgautherie)]

Comment on attachment 621414 [details] [diff] [review]
(Fv1) browser_popupNotification.js: Port |Bug 641892 - Support showing multiple popup notification icons at the same time|
[Checked in: Comment 27]

http://hg.mozilla.org/comm-central/rev/7715bb3d4e2d

[Approval Request Comment]
No risk, test-only.

Comment 28

[Serge Gautherie (:sgautherie)]

Comment on attachment 622585 [details] [diff] [review]
(Cv2) browser_popupNotification.js: Port |Bug 617553 - Doorhanger for add-on installation failures inside the AOM has no parent with chrome hidden|, Port patch Gv1-FF
[Checked in: See comments 23+28, 30, 34(*2)]

http://hg.mozilla.org/comm-central/rev/fa888360b067
(Hv1) browser_popupNotification.js: Copy missed nit from patch Gv1-FF

Comment 29

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1337348393.1337352489.23433.gz&fulltext=1
WINNT 5.2 comm-central-trunk debug test mochitest-other on 2012/05/18 06:39:53

V.Fixed

Comment 30

[Serge Gautherie (:sgautherie)]

Comment on attachment 622585 [details] [diff] [review]
(Cv2) browser_popupNotification.js: Port |Bug 617553 - Doorhanger for add-on installation failures inside the AOM has no parent with chrome hidden|, Port patch Gv1-FF
[Checked in: See comments 23+28, 30, 34(*2)]

http://hg.mozilla.org/releases/comm-beta/rev/cb351f3a6c59

Comment 31

[Serge Gautherie (:sgautherie)]

Comment on attachment 621412 [details] [diff] [review]
(Dv1) Port |Bug 587587 - Always dismiss popup notifications when they are hidden, and add "removeOnDismissal" option to show()|
[Checked in: See comments 24, 31, 35]

http://hg.mozilla.org/releases/comm-beta/rev/71e5441743eb

Comment 32

[Serge Gautherie (:sgautherie)]

Comment on attachment 621413 [details] [diff] [review]
(Ev1) Port |Bug 670851 - browser_bug519216.js, browser_popupNotification.js and browser_library_middleclick.js cause exception in tabbrowser.xml|
[Checked in: Comments 26, 32, 36]

http://hg.mozilla.org/releases/comm-beta/rev/6024971673a8

Comment 33

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Beta/1337595251.1337600055.1708.gz
WINNT 5.2 comm-beta debug test mochitest-other on 2012/05/21 03:14:11

seamonkey2.10: verified.

Comment 34

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 622585 [details] [diff] [review]
(Cv2) browser_popupNotification.js: Port |Bug 617553 - Doorhanger for add-on installation failures inside the AOM has no parent with chrome hidden|, Port patch Gv1-FF
[Checked in: See comments 23+28, 30, 34(*2)]

http://hg.mozilla.org/releases/comm-aurora/rev/3a6c8ae58492
http://hg.mozilla.org/releases/comm-aurora/rev/b091e8c8bda2

Comment 35

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 621412 [details] [diff] [review]
(Dv1) Port |Bug 587587 - Always dismiss popup notifications when they are hidden, and add "removeOnDismissal" option to show()|
[Checked in: See comments 24, 31, 35]

http://hg.mozilla.org/releases/comm-aurora/rev/b1c25eeb11dd

Comment 36

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 621413 [details] [diff] [review]
(Ev1) Port |Bug 670851 - browser_bug519216.js, browser_popupNotification.js and browser_library_middleclick.js cause exception in tabbrowser.xml|
[Checked in: Comments 26, 32, 36]

http://hg.mozilla.org/releases/comm-aurora/rev/d869f61c3d6a

Comment 37

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Aurora/1338445001.1338449007.28783.gz&fulltext=1
OS X 10.6 comm-aurora debug test mochitest-other on 2012/05/30 23:16:41

seamonkey2.11a2: verified.