498620 - Saved search folder is not visible in left folder pane or open-able in right pane immediately after saving

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090615 Firefox/3.5 ID:20090615154529

When you run a search inside the Library and save this search the folder is not shown in the left folder pane immediately. You have to reopen the Library to have the search visible there.

Steps:
1. Open Library and search for "Mozilla"
2. Click Save button and name it "Mozilla"
3. Observe the Bookmarks menu folder inside the left pane

After step 3 no saved search folder called "Mozilla" is visible in the left pane until you have reopened the Library.

This is a regression against Firefox 3.0.x.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

A quick check showed me that this regressed after Firefox 3.5 Beta 4.

Comment 2

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

i could guess some change to navHistoryResults.cpp, if that ends up being a perf improvement we could simply backout it from 1.9.1.

Comment 3

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

i suspect bug 494371. need to check builds still though.

Comment 4

[Alice0775 White]

Works:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/5cd96b6afea3
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090604 Shiretoko/3.5pre ID:20090604184627

Broken:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/010761f9d36b
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090604 Shiretoko/3.5pre ID:20090604213224

Pushlog:
http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?fromchange=5cd96b6afea3&tochange=010761f9d36b

Comment 5

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

yeah i won the bet, thanks Alice.
We could backout the perf fix from 1.9.1 if we want

Comment 6

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

this is minor since is an update problem but the query should appear when the view is loaded again... so we will most likely fix this in a maintenance release.

Comment 7

[Drew Willcoxon :adw]

Can't open the search folder in the right pane by double-clicking it either.  Bug 494371:

Works:
http://hg.mozilla.org/mozilla-central/rev/07971f68e728

Doesn't:
http://hg.mozilla.org/mozilla-central/rev/7c80b01e757b

Comment 8

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

it's my regression, so taking. 
Both issues are visible only for newly created searches.

Comment 9

[Drew Willcoxon :adw]

If I close the library and reopen, it works as expected.  Also,

1) Since the left pane doesn't display the search folder, it doesn't select it automatically and show its contents in the right pane like it should.
2) I noticed that the first time I save a search after the browser has been opened and I click on All Bookmarks in the left pane, nothing changes in the right pane -- it still shows the search.  When I click somewhere again in the left pane, the view updates as it should.

Comment 10

[Drew Willcoxon :adw]

(In reply to comment #9)
> 1) Since the left pane doesn't display the search folder, it doesn't select it
> automatically and show its contents in the right pane like it should.

Immediately after the search is saved, I mean.

> 2) I noticed that the first time I save a search after the browser has been
> opened and I click on All Bookmarks in the left pane, nothing changes in the
> right pane -- it still shows the search.  When I click somewhere again in the
> left pane, the view updates as it should.

This is true for whichever folder was selected in the left pane when I started the search.

Comment 11

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: patch v1.0

patch with a browser-chrome test, this is in addition to the test in bug 494372, and checks Library left pane view.

Comment 12

[Drew Willcoxon :adw]

I just tried dragging a tag folder to the Bookmarks Toolbar folder as a part of FFT testing, and it exhibited the same problems.

Comment 13

[Dietrich Ayala (:dietrich)]

(In reply to comment #12)
> I just tried dragging a tag folder to the Bookmarks Toolbar folder as a part of
> FFT testing, and it exhibited the same problems.

does the attached patch address this scenario?

Comment 14

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Yes, the attached patch handles that scenario. mainly i was returning too early onItemAdded, queries are uri nodes, but must be handled as containers.

Comment 15

[Dietrich Ayala (:dietrich)]

Comment on attachment 383758 [details] [diff] [review]
patch v1.0


>diff --git a/browser/components/places/tests/browser/browser_library_views_liveupdate.js b/browser/components/places/tests/browser/browser_library_views_liveupdate.js

any shared code with the other live-update test should be moved to the header


> nsNavHistoryFolderResultNode::OnItemAdded(PRInt64 aItemId,
>                                           PRInt64 aParentFolder,
>                                           PRInt32 aIndex,
>                                           PRUint16 aItemType)
> {
>   NS_ASSERTION(aParentFolder == mItemId, "Got wrong bookmark update");
> 
>+  PRBool excludeItems = (mResult && mResult->mRootNode->mOptions->ExcludeItems()) ||
>+                        (mParent && mParent->mOptions->ExcludeItems()) ||
>+                        mOptions->ExcludeItems();
>+

hm, i wonder how many of these there are. maybe should create a container helper for this at some point

r=me

Comment 16

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

http://hg.mozilla.org/mozilla-central/rev/b4cf38333c44

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed on 1.9.2 with builds on Windows and OS X like with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a2pre) Gecko/20090825 Namoroka/3.6a2pre ID:20090825033555

Comment 18

[Gervase Markham [:gerv]]

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