Closed Bug 481560 Opened 16 years ago Closed 16 years ago

tab closes and location bar clears before the window closes

Categories

(Firefox :: Tabbed Browser, defect)

Product:
Firefox
Component:
Tabbed Browser
Version:
3.5 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 3.6a1

People

(Reporter: sayrer, Assigned: dao)

References

Details

(Keywords: perf, regression, verified1.9.1, Whiteboard: [calm-ui])

Attachments

(1 file, 3 obsolete files)

patch
(deleted), patch
Gavin
: review+
beltzner
: approval1.9.1+
Details | Diff | Splinter Review
str: 1.) visit an SSL site 2.) close window with keyboard command (cmd-w, etc)
Strange. We also unload the document and clear the location bar here. This might be marginally more visible with the SSL UI, but isn't a regression from bug 480357. Change the pref back to 0 and you'll still see the unload/clear behaviour before window close, so the SSL indicator is just part of a larger bit of weirdness. We definitely close a window much slower now, in 3.0 this is basically instantaneous. Also happens on non-SSL sites. cc-ing dao, since he's done a bunch of tab stuff, wondering if there's a change that's responsible for this alteration of behaviour. Suspect we're doing something like clearing the existing tab before window.close() now...
No longer blocks: 480357
Component: Location Bar and Autocomplete → Tabbed Browser
Keywords: regressionwindow-wanted
QA Contact: location.bar → tabbed.browser
Summary: SSL tld display reverts to normal before the window closes → tab closes and location bar clears before the window closes
(In reply to comment #1) > Suspect we're doing > something like clearing the existing tab before window.close() now... That's right, it was needed for cases such as dragging the last tab to a different window.
... but I think that can be special-cased. I'll look into that.
Assignee: nobody → dao
Keywords: regressionwindow-wanted
OS: Mac OS X → All
Hardware: x86 → All
Attached patch patch (obsolete) (deleted) — — Splinter Review
Attachment #365644 - Flags: review?(gavin.sharp)
Blocks: 456002
Depends on: 462673
Keywords: perf, regression
Version: unspecified → 3.1 Branch
Flags: blocking-firefox3.1?
should have a perf test for this
Flags: in-testsuite?
Don't think it blocks release, but definitely wanted. Thanks for putting together a patch, Dao.
Flags: blocking-firefox3.1? → blocking-firefox3.1-
Blocks: 394759
Blocks: 483566
Attached patch patch (obsolete) (deleted) — — Splinter Review
test added
Attachment #365644 - Attachment is obsolete: true
Attachment #369069 - Flags: review?(gavin.sharp)
Attachment #365644 - Flags: review?(gavin.sharp)
Why does this behavior need to be optional (i.e. why do you need aCloseWindowFastpath)? I don't see offhand why other _beginRemoveTab callers wouldn't want this behavior.
It would regress bug 456002 otherwise.
Attached patch patch (obsolete) (deleted) — — Splinter Review
updated to latest trunk
Attachment #369069 - Attachment is obsolete: true
Attachment #369344 - Flags: review?(gavin.sharp)
Attachment #369069 - Flags: review?(gavin.sharp)
Attached patch patch (deleted) — — Splinter Review
updated to latest trunk
Attachment #369344 - Attachment is obsolete: true
Attachment #370281 - Flags: review?(gavin.sharp)
Attachment #369344 - Flags: review?(gavin.sharp)
Attachment #370281 - Flags: review?(gavin.sharp) → review+
Comment on attachment 370281 [details] [diff] [review] patch Ah, right. Seems like we should still be able to skip some of the work in endRemoveTab in the window-closing cases, but no need to fix that here.
http://hg.mozilla.org/mozilla-central/rev/aaff6f0f76c1
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.6a1
Attachment #370281 - Flags: approval1.9.1?
Flags: in-testsuite? → in-testsuite+
It looks like this patch is causing or exacerbating intermittent Linux test failures in test_Browser.js. TEST-PASS | chrome://mochikit/content/browser/browser/fuel/test/browser_Browser.js | Checking index after moving tab TEST-PASS | chrome://mochikit/content/browser/browser/fuel/test/browser_Browser.js | Checking event handler for tab move TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/fuel/test/browser_Browser.js | Timed out
That test was broken before and should have been disabled a while ago -- see bug 458688. I did that now.
Attachment #370281 - Flags: approval1.9.1? → approval1.9.1+
Whiteboard: [calm-ui]
verified FIXED when at https://bugzilla.mozilla.org on builds: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090604 Shiretoko/3.5pre (.NET CLR 3.5.30729) ID:20090604045218 and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090604 Minefield/3.6a1pre (.NET CLR 3.5.30729) ID:20090604042555
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: