Closed
Bug 145181
Opened 23 years ago
Closed 23 years ago
Tabbed browser is broken.
Categories
(SeaMonkey :: Tabbed Browser, defect)
SeaMonkey
Tabbed Browser
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: dylang, Assigned: rpotts)
References
Details
(Keywords: regression)
Attachments
(2 files, 1 obsolete file)
|
2.15 KB,
patch
|
bryner
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
|
7.85 KB,
patch
|
Details | Diff | Splinter Review |
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.
Comment 1•23 years ago
|
||
Reproduced on SunOS (SPARC) and Linux (x86). I'm pretty sure this was
introduced within the last 24hrs.
Comment 2•23 years ago
|
||
Confirming on linux trunk 2002051622.
No problem with 2002051522
Severity: normal → major
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: PC → All
Comment 3•23 years ago
|
||
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]
Comment 5•23 years ago
|
||
Comment 6•23 years ago
|
||
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. ***
Comment 8•23 years ago
|
||
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+
Comment 9•23 years ago
|
||
Someone will have to take over my patch (this was a
quick-and-very-dirty-copy-paste patch inspired from bug 46856)
Comment 10•23 years ago
|
||
this isn't a smoketest blocker, since we don't smoketest for this. Please remove
the smoketest keyword.
Updated•23 years ago
|
Keywords: regression
| Assignee | ||
Comment 13•23 years ago
|
||
Attachment #84033 -
Attachment is obsolete: true
| Assignee | ||
Comment 14•23 years ago
|
||
I believe this patch should fix the issues... just waiting for reviews to check in.
-- rick
Comment 15•23 years ago
|
||
Comment on attachment 84094 [details] [diff] [review]
Another patch (hopefully better)...
sr=jst
Attachment #84094 -
Flags: superreview+
| Assignee | ||
Comment 16•23 years ago
|
||
*** Bug 145231 has been marked as a duplicate of this bug. ***
Comment 17•23 years ago
|
||
Comment on attachment 84094 [details] [diff] [review]
Another patch (hopefully better)...
r=bryner
Attachment #84094 -
Flags: review+
Comment 18•23 years ago
|
||
*** Bug 145379 has been marked as a duplicate of this bug. ***
| Assignee | ||
Comment 19•23 years ago
|
||
checked in patch to the trunk.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
| Reporter | ||
Comment 20•23 years ago
|
||
2002051708 still broken....
When will this show up in the dailies?
Comment 21•23 years ago
|
||
#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.
Comment 22•23 years ago
|
||
Confirmed fixed in todays CVS build 2002051809.
Comment 23•23 years ago
|
||
*** Bug 145451 has been marked as a duplicate of this bug. ***
Comment 24•23 years ago
|
||
*** Bug 145447 has been marked as a duplicate of this bug. ***
Comment 25•23 years ago
|
||
*** Bug 145442 has been marked as a duplicate of this bug. ***
Comment 26•23 years ago
|
||
*** Bug 145446 has been marked as a duplicate of this bug. ***
Comment 27•23 years ago
|
||
*** Bug 145424 has been marked as a duplicate of this bug. ***
Comment 28•23 years ago
|
||
Confirmed: fixed in 2002051808. Nice job guys!
Comment 30•23 years ago
|
||
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 → ---
Comment 31•23 years ago
|
||
N.B. I'm not 100% sure that I'm using the right masks for each notification.
| Assignee | ||
Comment 32•23 years ago
|
||
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
Comment 33•23 years ago
|
||
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).
| Assignee | ||
Comment 34•23 years ago
|
||
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
Comment 35•23 years ago
|
||
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.
Comment 36•23 years ago
|
||
Please file a new bug for that issue.
Comment 37•23 years ago
|
||
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.
Comment 38•23 years ago
|
||
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 ago → 23 years ago
Resolution: --- → FIXED
Comment 39•23 years ago
|
||
vrfy'd fixed on recent branch and trunk builds.
Status: RESOLVED → VERIFIED
Updated•17 years ago
|
Product: Core → SeaMonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•