Open Bug 429287 Opened 14 years ago Updated 6 years ago

Tabbed browser should respond to DOMModalDialogClosed by restoring previously selected tab

Categories

(Firefox :: Tabbed Browser, enhancement)

enhancement
Not set
normal

Tracking

()

ASSIGNED

People

(Reporter: timeless, Assigned: timeless)

Details

(Whiteboard: [timeless: need to address comment 7/11])

Attachments

(1 file)

It currently respond to DOMWillOpenModalDialog, it should store the open tab and then restore it.
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #315946 - Flags: superreview?(gavin.sharp)
Attachment #315946 - Flags: review?(neil)
i believe that Camino already does something effectively like this
OS: Mac OS X → All
Hardware: Macintosh → All
Summary: Tabbed browser should respond to DOMModalDialogClosed → Tabbed browser should respond to DOMModalDialogClosed by restoring previously selected tab
I found this from Bug 405239 (https://bugzilla.mozilla.org/show_bug.cgi?id=405239) which is marked as "WONTFIX" even though a lot of people seem to dislike this new "feature" of FF3.  I think this sounds like the sanest way of dealing with this.  The current changing of tab focus with each dialog popup is very annoying (FF2 did not have this problem).
In lieu of 405239 getting fixed (hopefully by a 'tab bar' like the remember-password ui), this would certainly help me.
I was just preparing to revert the FF2 to get away from the frustrating behaviour of switching the tabs that FF3 does, when I found this thread.

It looks like a very good solution was submitted by timeless in April, and I was wondering if or when this would be available in FF3?  I don't think there is anyway to just apply this to an existing installation, right?

I have no desire to allow all sites to store permanent cookies, but some sites I really do want to allow it.  The 'ask me' is the only viable option I can find right now.

Thanks for a great product.
if i understand you right, there's other option:

"I have no desire to allow all sites to store permanent cookies"
privacy - cookies - keep until i close firefox

"but some sites I really do want to allow it"
privacy - cookies - exceptions

do NOT let "clear private data" deal cookies though, it doesn't respect this "exceptions"
Attachment #315946 - Flags: superreview?(gavin.sharp) → review+
Comment on attachment 315946 [details] [diff] [review]
handle paired event

>Index: mozilla/browser/base/content/tabbrowser.xml

>+      <handler event="DOMModalDialogClosed" phase="capturing">

>+          if (!this.mDialogStack.length)

this.mModalDialogStack

Test coverage for this shouldn't be very hard. You can open tabs and should be able to triggers alert()s from content with the browser chrome test harness.
http://developer.mozilla.org/en/Browser_chrome_tests
http://mxr.mozilla.org/seamonkey/source/browser/base/content/test/
I'm not a code contributor to either project, but my understanding is that Camino (the Mac specific offshoot of Mozilla) already does this.  It shouldn't be any harder to put this in as a XUL feature compared with a Cocoa one, given some of the code snippets already posted to this bug...
My bad. Missed timeless's comment above
Flags: blocking-firefox3.1?
Not blocking, but Neil, can we get a review?
Flags: blocking-firefox3.1? → blocking-firefox3.1-
Comment on attachment 315946 [details] [diff] [review]
handle paired event

>+          this.mModalDialogStack = [];
Make this a <field> please.

>+        <![CDATA[
>+          if (!event.isTrusted)
>+            return;
>+          if (!this.mDialogStack.length)
>+            return;
>+          if (this.selectedTab != this.mModalDialogStack[0].newTab)
>+            return;
>+          var pair = this.mModalDialogStack.shift();
>+          this.selectedTab = pair.oldTab;
Assuming that you do need this protection (I think thread manager allows you to easily post multiple modal dialogs against the same window) how do you propose to deal with the leaks you'll cause when modal dialogs are closed in the wrong order?
(In reply to comment #7)
> Test coverage for this shouldn't be very hard. You can open tabs and should be
> able to triggers alert()s from content with the browser chrome test harness.
How do you close them again ;-)
You can use a polling timer that uses the window watcher to find commonDialogs (see e.g. toolkit/components/passwordmgr/test/prompt_common.js ).
Please do not implement this suggestion.  Please mark this as WONTFIX.

Bug 405239 was finally fixed, which reverts to FF2 feature of "not switching focus to the other window wanting a cookie."  If you fix this bug, you will reintroduce the concept of switching to another background tab whenever it wants to set a cookie.  It is not sufficient to retain the current tab context, switch to the tab wanting a cookie for the dialog box, then revert to the previous tab.  The root of the annoyance is the focus switching in the first place.

At no point should a program switch focus for any reason other than explicit request of the user.  A background window wishing to set a cookie is a prime example of a case where the user did not explicitly request the focus to be stolen away.
I think this bug now applies to everything *except* the cookie dialog.
The fix for bug 405239 made the cookie dialog avoid setting a parent at all, which means this patch won't affect it.
Whiteboard: [timeless: need to address comment 11]
Whiteboard: [timeless: need to address comment 11] → [timeless: need to address comment 7/11]
Comment on attachment 315946 [details] [diff] [review]
handle paired event

Clearing old review request. It seems comment 7 and comment 11 still need to be addressed anyways before this bug can proceed.
Attachment #315946 - Flags: review?(neil)
Comment on attachment 315946 [details] [diff] [review]
handle paired event

I will clear gavin's r+ as well since this shouldn't appear as it is ready to land and will need to be rebased.
Attachment #315946 - Flags: review+
You need to log in before you can comment on or make changes to this bug.