Closed Bug 371074 Opened 18 years ago Closed 18 years ago

Full rebuild isn't always triggered when excludeQueries is set

Categories

(Firefox :: Bookmarks & History, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 3 alpha6

People

(Reporter: asaf, Assigned: dietrich)

References

Details

Attachments

(2 files, 1 obsolete file)

When excludeQueries is set for a places tree, we're supposed to do full rebuild rather than incremental update, apparently we sometimes do neither. See bug 366136.
The only remaining tree which set this is the organizer left pane, thus this should still block Fx3 but not turning places-bookmarks on.
No longer blocks: 370099
Flags: blocking-firefox3?
in bug #380495, onemen.one wrote: 1. open Bookmark Manager 2. change title to folder that is visible in the left pane Result: the folder title changed for all views except the left pane I'm not able to reproduce that with my Mac trunk debug bug (Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a5pre) Gecko/20070508 Minefield/3.0a5pre). onemen.one, which build are you using?
> 20070508 whoops, my mac build is older than I thought. Updating and rebuilding before trying onemen.one's steps.
> I'm not able to reproduce that with my Mac trunk debug bug I'm able to reproduce this with up to date windows XP build.
Flags: blocking-firefox3? → blocking-firefox3+
Target Milestone: --- → Firefox 3 alpha6
Attached patch fix v1 (obsolete) (deleted) — Splinter Review
was kicking out of onItemChanged for folders when excludeItems was set.
Attachment #267927 - Flags: review?(mano)
Comment on attachment 267927 [details] [diff] [review] fix v1 The correct check here is if (mOptions->ExcludeItems() && node->IsURI()) otherwise you would bail out for query-items. r=mano with that.
Attachment #267927 - Flags: review?(mano) → review+
hrm, there are few more places like this which needs to be fixed. I'm not sure whether the original bug is fixed by this patch, you may want to back-out the fix for bug 366136 and test. Also, I cannot recall whether we hide separator when this option is set. If so, my proposed check needs a little extension. :-/
Comment on attachment 267927 [details] [diff] [review] fix v1 Thus I would like to review this again.
Attachment #267927 - Flags: review+ → review-
IIRC the original bug was about the new-item case actually.
(In reply to comment #8) > hrm, there are few more places like this which needs to be fixed. > i looked at the other event handlers for nsNavHistoryFolderResultNode, and they seemed to have this fix (but using !IsFolder, not IsURI). Is that what you're referring to? > I'm not sure whether the original bug is fixed by this patch, you may want to > back-out the fix for bug 366136 and test. Also, I cannot recall whether we hide > separator when this option is set. If so, my proposed check needs a little > extension. :-/ > i reverted 366136, and this patch still fixes the symptom.
Attached patch fix v2 (deleted) — Splinter Review
fix mano's comments
Attachment #267927 - Attachment is obsolete: true
Attachment #268055 - Flags: review?(mano)
Comment on attachment 268055 [details] [diff] [review] fix v2 r=mano.
Attachment #268055 - Flags: review?(mano) → review+
Checking in toolkit/components/places/src/nsNavHistoryResult.cpp; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryResult.cpp,v <-- nsNavHistoryResult.cpp new revision: 1.101; previous revision: 1.100 done
Flags: in-testsuite?
Attached patch test (deleted) — Splinter Review
Attachment #268208 - Flags: review?(mano)
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Comment on attachment 268208 [details] [diff] [review] test * You don't need to close the container manually. * The file/test name should be limited to something like test_bug 371074, for our own sanity. r=mano otherwise.
Attachment #268208 - Flags: review?(mano) → review+
Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a7pre) Gecko/200708020808 Minefield/3.0a7pre.
Status: RESOLVED → VERIFIED
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

Created:
Updated:
Size: