Closed Bug 424444 Opened 17 years ago Closed 17 years ago

Sidebar bookmarks folders do not expand when reopening the sidebar

Categories

(Firefox :: Bookmarks & History, defect, P1)

Product:
Firefox
Component:
Bookmarks & History
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 3 beta5

People

(Reporter: ltsnow, Assigned: smaug)

References

Details

(Keywords: regression, Whiteboard: [has patch][has reviews][has approval])

Attachments

(2 files)

bring back the old behavior
1.63 KB, patch
mconnor
: review+
mconnor
: approval1.9b5+
Details | Diff | Splinter Review
+fix for the test
3.91 KB, patch
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008032103 Firefox/3.0b5pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008032103 Firefox/3.0b5pre

If you open the sidebar after first starting Minefield the bookmark folders will expand or collapse.  But if you close the sidebar you can no longer expand the bookmark folders.  This is true with a new or old profile.

I have isolated the problem to build 20080321_0812_firefox-3.0b5pre.en-US.win32.
Everything was fine with build 20080321_0537_firefox-3.0b5pre.en-US.win32.

Reproducible: Always

Steps to Reproduce:
1. Start Firefox and open sidebar. Bookmark folders expand OK. 
2. Close sidebar and reopen it. Now bookmark folders do not expand. 
3.
Actual Results:  
Bookmark folders do no collapse or expand.

Expected Results:  
Bookmark folders should expand and collapse.

As noted above, this is true with a new or old profile.
Version: unspecified → Trunk
I was able to reproduce this sporadically on Mac. What I did notice was that there seemed to be a distinct delay between when folders could be expanded or contracted.

STR:

1. Open bookmarks (or history) sidebar
2. Click on a folder to open it
3. Immediately click on the folder again to close it

Expected: folder closes

Actual: folder does not close! wait 2 seconds, try again: folder closes.

I'm not seeing any obvious culprits in the Places bugs checked in recently, so CCing Neil since this may be a widget issue.
Component: Bookmarks → Places
Flags: blocking-firefox3?
Keywords: regression
OS: Windows XP → All
Priority: -- → P1
QA Contact: bookmarks → places
Hardware: PC → All
Whiteboard: dupeme
Target Milestone: --- → Firefox 3 beta5
Status: UNCONFIRMED → NEW
Ever confirmed: true
To clarify: The delay problem is 100% reproduceable. The "unable to open" might just be the reporter experiencing the delay and not waiting to retry.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008032114 Minefield/3.0b5pre ID:2008032114

1) open Bookmark sidebar
2-1) click to open/close sub folder
2-2) not open/close
3) close sidebar, and reopen
4) sub folder open/close automatically
Nope when this happens the folder never opens.  
I mean never opens when waiting.  Closing and reopening the sidebar works like pla-moz states.
Dietrich, I am using a PC and what you are describing is not the problem I am
having.  When you open the sidebar you can't expand/open the folder after the
first time (of opening the sidebar).  Nothing happens--it's not a delay.

Additionally, what I think Pal-moz is saying:

If you click on a folder it does not open/expand.  However, if you close the sidebar and then reopen it the folder will be expanded.  Same thing works in reverse for collapsing/closing the folder.  Strange.
With one check-in in the regression range being mail/news and the other being debug only that leaves the bustage fix from bug 423874.
Blocks: 423874
Yes, I was thinking that the fix for bug 423874 was the culprit. 
Brian/Larry: what's the regression window you've identified?
The bustage fix was:

-    atom = fun->atom;
+    atom = FUN_ATOM(fun);

which doesn't seem like the sort of thing that would cause this problem.

Are we sure of that regression range?
That bustage fix affects code that is not build by default, and even when built only executes when we're run under dtrace.
I am absolutely certain that the regression was in build
(20080321_0812_firefox-3.0b5pre.en-US.win32).
Everything was fine with build (20080321_0537_firefox-3.0b5pre.en-US.win32).

I tested all 10 hourly builds from the 20th (all good) and now all 12 builds
from the 21st (first 4 good and all bad after build 0537).

I want to suspect bug 395609, but that was checked in at 04:18, and would have been part of the 05:37 build, would it not?
The checkout log from the 08:12 build was:

checkout start: Fri Mar 21 08:12:56 PDT 2008
cvs  -q -z 3  co   -D "03/21/2008 15:12 +0000" mozilla/client.mk mozilla/browser/config/mozconfig mozilla/browser/config/version.txt mozilla/build/unix/uniq.pl mozilla/calendar/sunbird/config/version.txt mozilla/mail/config/version.txt mozilla/suite/config/version.txt
make[1]: Entering directory `/e/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend'
cvs -q -z 3 co -P -r NSPR_4_7_1_BETA2 mozilla/nsprpub
cvs -q -z 3 co -P -r NSS_3_12_BETA3 mozilla/dbm mozilla/security/nss mozilla/security/coreconf mozilla/security/dbm

cvs -q -z 3 co -P -A -D 03/21/2008 15:12 +0000 -l mozilla/ mozilla/db mozilla/js mozilla/js/jsd mozilla/js/src
? mozilla/obj-fx-trunk
U mozilla/js/src/jsdtracef.c
cvs -q -z 3 co -P -A -D 03/21/2008 15:12 +0000 mozilla/README mozilla/accessible mozilla/browser mozilla/build mozilla/caps mozilla/chrome mozilla/config mozilla/content mozilla/db/mdb mozilla/db/mork mozilla/db/morkreader mozilla/db/sqlite3 mozilla/docshell mozilla/dom mozilla/editor mozilla/embedding mozilla/extensions mozilla/gfx mozilla/intl mozilla/ipc/ipcd mozilla/jpeg mozilla/js/jsd/idl mozilla/js/src/fdlibm mozilla/js/src/liveconnect mozilla/js/src/xpconnect mozilla/layout mozilla/memory/jemalloc mozilla/modules/lcms mozilla/modules/libbz2 mozilla/modules/libimg mozilla/modules/libjar mozilla/modules/libmar mozilla/modules/libpr0n mozilla/modules/libpref mozilla/modules/libreg mozilla/modules/libutil mozilla/modules/oji mozilla/modules/plugin mozilla/modules/staticmod mozilla/modules/zlib mozilla/netwerk mozilla/other-licenses/7zstub/firefox mozilla/other-licenses/atk-1.0 mozilla/other-licenses/branding/firefox mozilla/other-licenses/ia2 mozilla/parser mozilla/plugin/
 oji mozill
a/probes mozilla/profile mozilla/rdf mozilla/security/manager mozilla/storage mozilla/sun-java mozilla/testing/crashtest mozilla/testing/mochitest mozilla/toolkit mozilla/tools/elf-dynstr-gc mozilla/tools/test-harness mozilla/tools/update-packaging mozilla/uriloader mozilla/view mozilla/webshell mozilla/widget mozilla/xpcom mozilla/xpfe mozilla/xpinstall
U mozilla/dom/src/base/nsJSEnvironment.cpp
checkout finish: Fri Mar 21 08:21:47 PDT 2008

Well, the only change to nsJSEnvironment.cpp was a change to a debug string. So, uh, none of the changes between the two builds could possibly have caused this.
Larry's range was correct, but it won't be the first time that nothing not even in the proximity of the range could be blamed for the bug.
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1206103020&maxdate=1206112319

By the way, the 0537 build and also the 0354 build have both the ID 2008032103 (in compability.ini).
2008032106:

Closing the sidebar does not unload the document shown in the sidebar.
The document is only unloaded if another sidebar is opened.
disturbedite just submitted the following on my thread:

confirmed on linux as well.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b5pre) Gecko/2008032204 Minefield/3.0b5pre - Build ID: 2008032204
(In reply to comment #19) 
> Closing the sidebar does not unload the document shown in the sidebar.
> The document is only unloaded if another sidebar is opened.
If this causes the problem, then this is certainly a regression from bug 395609. 
Something needs to be fixed in chrome. Possibly related to bug 406697 too.

This should bring back the old behavior. I'm not sure if some other behavior
is wanted because of bug 406697, but at least here this doesn't regress 
bug 406697 - I can load bookmarked pages in sidebar.
Perhaps for b5 this is enough.
Attachment #311188 - Flags: review?(myk)
Flags: blocking-firefox3? → blocking-firefox3+
Attachment #311188 - Flags: review?(myk) → review?(mconnor)
Whiteboard: dupeme → [has patch][needs review mconnor]
Comment on attachment 311188 [details] [diff] [review]
bring back the old behavior

r+a1.9b5=mconnor

this is straightforward and should be quite safe.  tested and works as expected.

Olli, please land morning your time or let me know if you don't have time to land it.
Attachment #311188 - Flags: review?(mconnor)
Attachment #311188 - Flags: review+
Attachment #311188 - Flags: approval1.9b5+
Whiteboard: [has patch][needs review mconnor] → [has patch][has reviews][has approval]
Assignee: nobody → Olli.Pettay
Argh, the patch makes one browser test fail.
The failing test is for bug 409481.
Trying to find either better patch or possible "bug" in the testcase.
OK, seems like a bug in the testcase.
It is adding a closure, which then calls a function, as load event listener,
but trying to remove that function as load event listener.
Checked in
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
(In reply to comment #26)
> Created an attachment (id=311238) [details]
> +fix for the test
> 

Does this test fix resolve the issue raised in bug 421333?
Blocks: 421333
It is fixed.  Thanks to all.
This is not fixed for me.  No errors or messages in the Error Console.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b5pre) Gecko/2008032304 Minefield/3.0b5pre - Build ID: 2008032304
The patch with the fix for the test was checked in 05:58, which is after 
2008032304 build, I think.
verified with:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5) Gecko/2008032606 Firefox/3.0b5
and
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-GB; rv:1.9b5) Gecko/2008032604 Firefox/3.0b5
Status: RESOLVED → VERIFIED
Confirmed fixed with:

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b5pre) Gecko/2008032604 Minefield/3.0b5pre - Build ID: 2008032604
No longer blocks: 423874
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: