If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Port |Bug 59314 - Alerts should be content-modal, not window-modal| to SeaMonkey

NEW
Unassigned

Status

SeaMonkey
Tabbed Browser
7 years ago
6 years ago

People

(Reporter: sgautherie, Unassigned)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Reporter)

Updated

7 years ago
Blocks: 467530
(Reporter)

Updated

7 years ago
Blocks: 625038
(Reporter)

Updated

7 years ago
Depends on: 625114
(Reporter)

Comment 1

7 years ago
Created attachment 503215 [details] [diff] [review]
(Av1) Just port it, Remove an obsolete comment

This fixes bug 625038, after fixing bug 625114 too.
I'll land this when the latter is fixed only...

This code is mostly copied from FF, with some nits fixed.
The only points I wondered about:
*does the code belongs in navigator.js (and not browser.js)?
*should I s/gBrowser/getBrowser/ there?
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #503215 - Flags: review?(neil)
(Reporter)

Comment 2

7 years ago
Comment on attachment 503215 [details] [diff] [review]
(Av1) Just port it, Remove an obsolete comment

(In reply to comment #1)
> The only points I wondered about:

And "do we need an extra check or something to be safe on the mailnews side?".
(Reporter)

Comment 3

7 years ago
Created attachment 503235 [details] [diff] [review]
(Bv1-FF) Fix some nits
Attachment #503235 - Flags: review?(dolske)

Comment 4

7 years ago
Comment on attachment 503215 [details] [diff] [review]
(Av1) Just port it, Remove an obsolete comment

This is missing a swathe of XBL changes, both tabbrowser and notificationbox.

> 
> 
Nit: don't need two consecutive blank lines

>+  let foundBrowser = gBrowser.getBrowserForDocument(aWindow.document);
[What happens if a frame alerts?]

>+            let browser = aBrowser || this.mCurrentBrowser;
Nit: should put tab here, it doesn't change between prompts.

>+                let tab = self._getTabForContentWindow(browser.contentWindow);
Someone mentioned on one of the session store bugs that we need a getTabForBrowser function (session store has an internal version) which Gavin suggested could use the uniqueID (browser.id == tab.linkedPanel).

>+                let count = parseInt(browser.getAttribute("tabmodalPromptShowing")) - 1;
Nit: The - operator already does a parse (ok, so it's a parseFloat...)

>+              listPrompts : function(aPrompt) {
[Unused parameter?]

>+                // NodeList --> real JS array
There's a function for that, although I forget it offhand.
Attachment #503215 - Flags: review?(neil) → review-
Comment on attachment 503235 [details] [diff] [review]
(Bv1-FF) Fix some nits

As far as I can see this is just nitpicky style churn.
Attachment #503235 - Flags: review?(dolske) → review-
(Reporter)

Updated

7 years ago
Attachment #503235 - Attachment is obsolete: true
(Reporter)

Updated

7 years ago
Blocks: 626294
Looks like Serge dropped this, and it is of HIGH value to our users, removes our DOS strike-area.

Firefox has had this feature since Fx4.

Ewong, does this look like something you'd be capable/willing to take on, (it is more advanced than your average bug)
Assignee: sgautherie.bz → nobody

Updated

6 years ago
Status: ASSIGNED → NEW
(Reporter)

Comment 7

6 years ago
(In reply to comment #6)
> Looks like Serge dropped this,

Yes. Iiuc, this +/- depend(ed) on bug 625114, which in turn...
These seemed too complicated wrt my limited skill in this area.
(And I currently don't have much free time (to spend on Moz/SM) :-|)
(Reporter)

Updated

6 years ago
Blocks: 736905

Comment 8

6 years ago
>>+                // NodeList --> real JS array
> There's a function for that, although I forget it offhand.
Array.slice() ?
You need to log in before you can comment on or make changes to this bug.