Last Comment Bug 752216 - Port |Bug 641892 - Support showing multiple popup notification icons at the same time| to SeaMonkey
: Port |Bug 641892 - Support showing multiple popup notification icons at the s...
Status: VERIFIED FIXED
[fixed in SM 2.10: Cv2, Dv1a, Ev1; SM...
:
Product: SeaMonkey
Classification: Client Software
Component: UI Design (show other bugs)
: Trunk
: All All
: P2 major (vote)
: seamonkey2.12
Assigned To: Serge Gautherie (:sgautherie)
:
Mentors:
http://mxr.mozilla.org/comm-central/f...
Depends on: 587587 595810 617553 641892 670851
Blocks: SmTestFail
  Show dependency treegraph
 
Reported: 2012-05-05 07:05 PDT by Serge Gautherie (:sgautherie)
Modified: 2012-05-31 03:24 PDT (History)
3 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
verified
verified


Attachments
(Av1) Update notification-anchor-icon rules in navigator.css [Checked in: Comment 4] (1.38 KB, patch)
2012-05-05 07:52 PDT, Tetsuharu OHZEKI [:tetsuharu] [UTC+9]
neil: review+
bugzillamozillaorg_serge_20140323: feedback+
Details | Diff | Splinter Review
(Bv1-FF) browser_popupNotification.js: Fix nits [Checked in: Comment 15] (6.04 KB, patch)
2012-05-06 06:10 PDT, Serge Gautherie (:sgautherie)
gavin.sharp: review+
Details | Diff | Splinter Review
(Cv1) browser_popupNotification.js: Port |Bug 617553 - Doorhanger for add-on installation failures inside the AOM has no parent with chrome hidden| (2.17 KB, patch)
2012-05-06 06:17 PDT, Serge Gautherie (:sgautherie)
neil: review-
Details | Diff | Splinter 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] (5.49 KB, patch)
2012-05-06 06:25 PDT, Serge Gautherie (:sgautherie)
neil: review+
bugspam.Callek: approval‑comm‑aurora+
bugspam.Callek: approval‑comm‑beta+
Details | Diff | Splinter 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] (2.05 KB, patch)
2012-05-06 06:31 PDT, Serge Gautherie (:sgautherie)
neil: review+
bugspam.Callek: approval‑comm‑aurora+
bugspam.Callek: approval‑comm‑beta+
Details | Diff | Splinter Review
(Fv1) browser_popupNotification.js: Port |Bug 641892 - Support showing multiple popup notification icons at the same time| [Checked in: Comment 27] (3.84 KB, patch)
2012-05-06 06:34 PDT, Serge Gautherie (:sgautherie)
neil: review+
Details | Diff | Splinter Review
(Gv1-FF) browser_popupNotification.js: Use locationbar to simplify test 19 [Checked in: Comment 21] (2.74 KB, patch)
2012-05-07 06:36 PDT, Serge Gautherie (:sgautherie)
gavin.sharp: review+
Details | Diff | Splinter 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)] (1.86 KB, patch)
2012-05-09 17:13 PDT, Serge Gautherie (:sgautherie)
neil: review+
bugspam.Callek: approval‑comm‑aurora+
bugspam.Callek: approval‑comm‑beta+
Details | Diff | Splinter Review

Description Serge Gautherie (:sgautherie) 2012-05-05 07:05:52 PDT
{
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] 2012-05-05 07:52:21 PDT
Created attachment 621296 [details] [diff] [review]
(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) 2012-05-05 08:33:09 PDT
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 neil@parkwaycc.co.uk 2012-05-05 09:51:06 PDT
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) 2012-05-05 12:22:05 PDT
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) 2012-05-06 06:10:09 PDT
Created attachment 621410 [details] [diff] [review]
(Bv1-FF) browser_popupNotification.js: Fix nits
[Checked in: Comment 15]

Back-port from SeaMonkey.
Comment 6 Serge Gautherie (:sgautherie) 2012-05-06 06:17:21 PDT
Created 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|

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) 2012-05-06 06:25:23 PDT
Created 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]

Trivial copy.
Comment 8 Serge Gautherie (:sgautherie) 2012-05-06 06:31:19 PDT
Created 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]

Trivial copy.
Comment 9 Serge Gautherie (:sgautherie) 2012-05-06 06:34:38 PDT
Created 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]

Trivial copy (with 2 ',' nits).
Comment 10 neil@parkwaycc.co.uk 2012-05-06 06:50:44 PDT
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 neil@parkwaycc.co.uk 2012-05-06 06:52:17 PDT
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) 2012-05-06 08:09:52 PDT
(In reply to neil@parkwaycc.co.uk 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 neil@parkwaycc.co.uk 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 neil@parkwaycc.co.uk 2012-05-06 09:37:48 PDT
(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 neil@parkwaycc.co.uk 2012-05-06 09:39:23 PDT
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) 2012-05-06 12:47:04 PDT
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) 2012-05-07 06:36:35 PDT
Created attachment 621578 [details] [diff] [review]
(Gv1-FF) browser_popupNotification.js: Use locationbar to simplify test 19
[Checked in: Comment 21]

Per comment 13.
Comment 17 neil@parkwaycc.co.uk 2012-05-07 06:45:47 PDT
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) 2012-05-07 11:08:40 PDT
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: gavin@gavinsharp.com] 2012-05-07 13:28:18 PDT
Does the test still fail if you break the relevant functionality?
Comment 20 Serge Gautherie (:sgautherie) 2012-05-07 16:47:22 PDT
(In reply to :Gavin Sharp (use gavin@gavinsharp.com 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) 2012-05-09 17:02:45 PDT
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) 2012-05-09 17:13:07 PDT
Created 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)]

Cv1, with comment 13 suggestion(s).
Comment 23 Serge Gautherie (:sgautherie) 2012-05-10 06:29:31 PDT
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) 2012-05-10 06:30:43 PDT
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) 2012-05-10 06:33:29 PDT
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) 2012-05-10 06:34:35 PDT
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) 2012-05-10 06:35:26 PDT
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) 2012-05-10 06:44:04 PDT
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) 2012-05-18 11:28:20 PDT
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) 2012-05-21 12:06:05 PDT
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) 2012-05-21 12:07:18 PDT
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) 2012-05-21 12:08:02 PDT
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) 2012-05-21 12:10:38 PDT
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) 2012-05-30 13:36:42 PDT
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) 2012-05-30 13:37:09 PDT
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) 2012-05-30 13:37:37 PDT
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) 2012-05-31 03:24:59 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.