Closed Bug 145181 Opened 23 years ago Closed 23 years ago

Tabbed browser is broken.

Categories

(SeaMonkey :: Tabbed Browser, defect)

defect
Not set
blocker

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: dylang, Assigned: rpotts)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

2002051621 Tabbed browsing is totally broken: new tabs won't load URLs (middle click), they won't close (close tab popup menu, X button, etc), and generally are very useless. Seems like a regression from an incomplete commit.
Reproduced on SunOS (SPARC) and Linux (x86). I'm pretty sure this was introduced within the last 24hrs.
Confirming on linux trunk 2002051622. No problem with 2002051522
Severity: normal → major
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: PC → All
This is what I get when I try to open a tab for the first time (and no tabs is opened): Error: uncaught exception: [Exception... "Not enough arguments [nsIWebProgress.addProgressListener]" nsresult: "0x80570001 (NS_ERROR_XPC_NOT_ENOUGH_ARGS)" location: "JS frame :: chrome://global/content/bindings/tabbrowser.xml#tabbrowser.enterTabbedMode() :: enterTabbedMode :: line 26" data: no] Subsequent tab opening gives this (and tabs open with the url as tab title): Error: uncaught exception: [Exception... "Not enough arguments [nsIWebProgress.addProgressListener]" nsresult: "0x80570001 (NS_ERROR_XPC_NOT_ENOUGH_ARGS)" location: "JS frame :: chrome://global/content/bindings/tabbrowser.xml#tabbrowser.addTab() :: addTab :: line 43" data: no]
caused by checkin for bug 46856 ?
Attached patch Patch (obsolete) — Splinter Review
The patch is working great for me. Thanks Gilles. Let's get this in quick, it's majorly annoying bug :)
*** Bug 145266 has been marked as a duplicate of this bug. ***
Severity: major → blocker
Keywords: smoketest
Comment on attachment 84033 [details] [diff] [review] Patch This only fixes the calls from the tabbed browser, the calls to the tabbed browser will expect to pass two arguments, and the tabbed browser will have to pass only the expected progress events.
Attachment #84033 - Flags: needs-work+
Someone will have to take over my patch (this was a quick-and-very-dirty-copy-paste patch inspired from bug 46856)
Blocks: 145231
this isn't a smoketest blocker, since we don't smoketest for this. Please remove the smoketest keyword.
removing smoketest kw.
Status: NEW → ASSIGNED
Keywords: smoketest
->rpotts
Assignee: jaggernaut → rpotts
Status: ASSIGNED → NEW
Keywords: regression
Blocks: 143200
Attachment #84033 - Attachment is obsolete: true
I believe this patch should fix the issues... just waiting for reviews to check in. -- rick
No longer blocks: 143200
Comment on attachment 84094 [details] [diff] [review] Another patch (hopefully better)... sr=jst
Attachment #84094 - Flags: superreview+
*** Bug 145231 has been marked as a duplicate of this bug. ***
Comment on attachment 84094 [details] [diff] [review] Another patch (hopefully better)... r=bryner
Attachment #84094 - Flags: review+
*** Bug 145379 has been marked as a duplicate of this bug. ***
checked in patch to the trunk.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
2002051708 still broken.... When will this show up in the dailies?
#20 The patch was checked in 05/17/2002 17:26, and your build is from 05/17/2002 08:00. A build from 05/18 should have this fixed.
Confirmed fixed in todays CVS build 2002051809.
*** Bug 145451 has been marked as a duplicate of this bug. ***
*** Bug 145447 has been marked as a duplicate of this bug. ***
*** Bug 145442 has been marked as a duplicate of this bug. ***
*** Bug 145446 has been marked as a duplicate of this bug. ***
*** Bug 145424 has been marked as a duplicate of this bug. ***
Confirmed: fixed in 2002051808. Nice job guys!
verified fixed
Status: RESOLVED → VERIFIED
Sorry, but I'm not convinced it's completely fixed. 1) If autohide is disabled then tabbed browser progress listener uses mask of first progress listener added to it when it listens to real browser, so missing out on all the progress that it and other listeners need. 2) tabbed browser sends all progress to all listeners, ignoring the masks
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
N.B. I'm not 100% sure that I'm using the right masks for each notification.
i guess, i don't understand the WebProgressListener mechanism in the tab-browser... i had assumed that it was simply a fan-out mechanism... and that ALL listeners would want to receive the same set of notifications... Do we *really* need to let some tabs receive different sets of notifications? If not, then i think it would be simplier to just ignore the 'aMask' passed into addProgressListener and pass in the mask-set that is necessary for all tabbed browsing (for backward compatability that could be NOTIFY_ALL). -- rick
It's not the tabs that want different notifications, it's the listeners. Within a tab the browser will generate various notifications, plus extra notifications are generated when changing tabs. Do you want to send these to all listeners? Even if so, you still need to ignore the one aMask case (line 788).
I guess, what i'm asking is if adding support for each listener to provide it's own mask will *actually be used*... Before the nsIWebProgress changes, each listener received ALL notifications... After the changes, will it be possible to limit the scope of the notifications? Or will we still need to pass in NOTIFY_ALL. If we have to use NOTIFY_ALL, then i'd be inclined to just pass that in (and ignore the aMask as you said at line 788)... This would avoid the extra overhead of tracking the masks (if we don't absolutely have to)... does this make sense? i'm still not sure i've got a handle on how these notifications are used :-) -- rick
Maybe related to the current bug (still present on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0rc3) Gecko/20020529 Debian/1.0rc3-1): you have to click and focus on a newly opened tab to get control-W to work. No click, no close.
Please file a new bug for that issue.
This patch has serious problems for me. The URLbar doesn't update and the Back/Forward buttons are continuously disabled. It other words, it needs more love. note: I used RC3 final on Windows 2000 as test platform.
Actually the code at line 788 shouldn't be there, so I'll change the resolution back to fixed and track my remaining issue in a new bug.
Status: REOPENED → RESOLVED
Closed: 23 years ago23 years ago
Resolution: --- → FIXED
vrfy'd fixed on recent branch and trunk builds.
Status: RESOLVED → VERIFIED
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: