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

Can't load the correct bookmark in sidebar after opened another page in it.

VERIFIED FIXED in Firefox 3 beta5

Status

()

Firefox
Bookmarks & History
P2
major
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: bobchao, Assigned: myk)

Tracking

({regression})

Trunk
Firefox 3 beta5
x86
All
regression
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 obsolete attachments)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007120204 Minefield/3.0b2pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007120204 Minefield/3.0b2pre

This bug is happened both on my XP and Linux (minefield build num is the same.)

If you loaded one bookmark in  sidebar before in the browser window, you can't load another page in it correctly -- the page loaded in sidebar will still be the previous one.

Reproducible: Always

Steps to Reproduce:
1. Add one bookmark (say, http://google.com) by dragging favicon to Bookmarks Toolbar, check "Load this bookmark in the sidebar" in the bookmark's Properties dialog.
2. Add another one bookmark (say, http://yahoo.com) by dragging favicon to Bookmarks Toolbar,  also check "Load this bookmark in the sidebar" in the bookmark's Properties dialog.
3. Click the 1st bookmark, the sidebar should open with Google loaded.
4. Close the sidebar.
5. Click the 2nd bookmark.
Actual Results:  
Google shows again.

Expected Results:  
Yahoo! should be loaded this time.

If you open another browser window and click the 2nd bookmark, it would be Yahoo! loaded correctly in sidebar, and then you can't load Google in the window's sidebar. This problem didn't happened in Firefox 2.
I'm seeing this is OS X as well with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b2pre) Gecko/2007120304 Minefield/3.0b2pre.

Comment 2

10 years ago
confirmed - notice after step 5 that the title of the sidebar is Yahoo! but the content remains Google. New Window behavior is funky as well.  

Requesting blocking Fx3 - we should be able to consecutively load sidebar loading bookmarks.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-firefox3?
Keywords: regression
Regression range for this is http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1196066040&maxdate=1196067059 so Bug 385224 could have caused this.
Blocks: 385224

Comment 4

10 years ago
I'm having this problem too. It first appeared in build 2007112605.
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P2
(Assignee)

Comment 5

10 years ago
Yup, this is a regression from bug 385224.  Looking...
Assignee: nobody → myk
(Assignee)

Comment 6

10 years ago
This is a regression from the change to browser.xml in bug 385224.  That change makes browsers cache their docShell objects to speed up things that access them.

That's OK for the browsers in the main tabbrowser, which retain the same docShell for their entire lifetime, as far as I can tell.  But it's not OK for the sidebar browser, which gets a new docShell every time you hide and show it.

And so the cache is stale the second time you open the sidebar, and the code in openWebPanel that is responsible for loading the new URL fails, although it does succeed in first updating the sidebar title, which is why the sidebar title is correct at the end of the testcase.

I know of two fixes to this bug:

1. Revert the change that added the docShell cache.  This will impact performance, although it's not clear how much.  The docShell cache was just one of a number of optimizations in the fix for bug 385224.

2. Extend the browser widget with a version that uses the cache and only bind the tabbrowser browsers to that version.

cc:ing some layout folks, though, in case there's a third solution (f.e. perhaps browser widgets shouldn't be getting new docShells every time they get hidden and reshown, and we can fix that bug instead).
Status: NEW → ASSIGNED
(Assignee)

Comment 7

10 years ago
Created attachment 295604 [details] [diff] [review]
fix 1: revert the optimization from bug 385224

Here's the first of the two possible fixes I identified in comment 6.  This fix just reverts the optimization from bug 385224, sacrificing that performance win to fix this bug.  This is the lower-risk fix.
Attachment #295604 - Flags: review?(mconnor)
(Assignee)

Comment 8

10 years ago
Created attachment 295605 [details] [diff] [review]
fix 2: special-case tabbrowser browsers

Here's the second of the two possible fixes I identified in comment 6.  This fix retains the performance optimization for the main tabbrowser widget while fixing this bug in the sidebar browser widget by making the cache apply specifically to browsers that are children of the #content node.  This is the higher-risk fix.
Attachment #295605 - Flags: review?(mconnor)
(Assignee)

Updated

10 years ago
Attachment #295605 - Attachment description: fix 1: special-case tabbrowser browsers → fix 2: special-case tabbrowser browsers
(Assignee)

Comment 9

10 years ago
Update: Neil Rashbrook on IRC says that XUL browser/iframe elements intentionally destroy their docShells when they get hidden, so that's not a bug.

A third fix might be to invalidate the cache when the element gets hidden, but it's not clear how to detect that the element has been hidden.  We could check on every unload/pagehide, but the cost of that might just obviate the perf gain from caching the docShell in the first place.
> Neil Rashbrook on IRC says that XUL browser/iframe elements
> intentionally destroy their docShells when they get hidden, so that's not a
> bug.

Actually, it is.  I was sure we have it filed, but I can't find it...  The docshell should really be owned by the binding, not the frame or something.  The current setup has a number of problems that would be fixed by that change.

That's not a 1.9 kind of change at this point, probably.
(Assignee)

Comment 11

10 years ago
Created attachment 295664 [details] [diff] [review]
fix 3: invalidate the docshell cache on browser hide

Per comment 9, here's the third possible fix.  This retains the docshell cache but invalidates it on pagehide if the pagehide is occurring because the browser has been hidden.  This should retain some of the performance win of the original fix that caused the regression, although it's not clear how much.  As a side-effect, it includes the typo fix followup patch in bug 407116.  This is the middling-risk fix.
Attachment #295664 - Flags: review?(mconnor)
(Assignee)

Comment 12

10 years ago
Created attachment 296089 [details] [diff] [review]
fix 3 unrotted
Attachment #295664 - Attachment is obsolete: true
Attachment #296089 - Flags: review?(mconnor)
Attachment #295664 - Flags: review?(mconnor)
Duplicate of this bug: 411890
Blocks: 406340
Blocks: 410559
Version: unspecified → Trunk
(In reply to comment #12)
> Created an attachment (id=296089) [details]
> fix 3 unrotted
> 

This fix looks pretty safe. This bug has been causing more than a few problems in extensions as well. We should get this problem resolved sooner than later with more extensions updating to Fx3.

At the very least we should get fix 1 landed (reverts the optimization from bug 385224) and see how much the pref regresses.
Duplicate of this bug: 410559

Updated

10 years ago
Duplicate of this bug: 415770

Updated

10 years ago
Duplicate of this bug: 417456
If it hasn't rotted, can we get that fix reviewed and checked-in for b4
Target Milestone: --- → Firefox 3 beta4

Comment 19

10 years ago
same on 3.0b3

on webpanel open:
Error: this.docShell is null
Source File: chrome://global/content/bindings/browser.xml
Line: 0

on webpanel closed:
Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIWebProgress.removeProgressListener]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://browser/content/web-panels.js :: unload :: line 87"  data: no]

on webpanel re-open:
Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIWebNavigation.document]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://global/content/bindings/browser.xml :: get_contentDocument :: line 0"  data: no]

result: opens last page from the first time it was open

version:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3) Gecko/2008020513 Firefox/3.0b3

Talked to bz and smaug about this on IRC today, they pointed out that bug 395609 will fix this by fixing the bug bz describes in comment 10. I've confirmed that the latest patch in that bug fixes this one, and that bug is a blocker, so we can just wait until that lands and avoid having to take any of the patches in this bug.
Depends on: 395609
Attachment #295604 - Attachment is obsolete: true
Attachment #295604 - Flags: review?(mconnor)
Attachment #295605 - Attachment is obsolete: true
Attachment #295605 - Flags: review?(mconnor)
Attachment #296089 - Attachment is obsolete: true
Attachment #296089 - Flags: review?(mconnor)
Target Milestone: Firefox 3 beta4 → Firefox 3

Updated

10 years ago
Duplicate of this bug: 421492

Updated

10 years ago
Whiteboard: [fixed by bug 395609]
Whiteboard: [fixed by bug 395609] → [will be fixed by bug 395609]

Updated

10 years ago
Blocks: 423392
Confirmed fixed on trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Whiteboard: [will be fixed by bug 395609]
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9b5pre) Gecko/2008032005 Minefield/3.0b5pre] (nightly) (W2Ksp4)

V.Fixed (behavior from comment 0, 1 strict warning and 2 exceptions from comment 19) by
{{
2008-03-21 04:18	Olli.Pettay%helsinki.fi 	...  	Bug 395609, r=roc+sicking, sr=sicking, a=beltzner
}}

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9b5pre) Gecko/2008032106 Minefield/3.0b5pre] (nightly) (W2Ksp4)
Status: RESOLVED → VERIFIED
No longer blocks: 410559
No longer blocks: 406340
Duplicate of this bug: 406340
Target Milestone: Firefox 3 → Firefox 3 beta5
You need to log in before you can comment on or make changes to this bug.