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)
Firefox
Bookmarks & History
Tracking
()
VERIFIED
FIXED
Firefox 3 alpha6
People
(Reporter: asaf, Assigned: dietrich)
References
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
asaf
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
asaf
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•18 years ago
|
||
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
Reporter | ||
Updated•18 years ago
|
Flags: blocking-firefox3?
Comment 3•18 years ago
|
||
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?
Comment 4•18 years ago
|
||
> 20070508
whoops, my mac build is older than I thought.
Updating and rebuilding before trying onemen.one's steps.
Comment 5•18 years ago
|
||
> 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.
Updated•18 years ago
|
Flags: blocking-firefox3? → blocking-firefox3+
Updated•18 years ago
|
Target Milestone: --- → Firefox 3 alpha6
Assignee | ||
Comment 6•18 years ago
|
||
was kicking out of onItemChanged for folders when excludeItems was set.
Attachment #267927 -
Flags: review?(mano)
Reporter | ||
Comment 7•18 years ago
|
||
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+
Reporter | ||
Comment 8•18 years ago
|
||
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. :-/
Reporter | ||
Comment 9•18 years ago
|
||
Comment on attachment 267927 [details] [diff] [review]
fix v1
Thus I would like to review this again.
Attachment #267927 -
Flags: review+ → review-
Reporter | ||
Comment 10•18 years ago
|
||
IIRC the original bug was about the new-item case actually.
Assignee | ||
Comment 11•18 years ago
|
||
(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.
Assignee | ||
Comment 12•18 years ago
|
||
fix mano's comments
Attachment #267927 -
Attachment is obsolete: true
Attachment #268055 -
Flags: review?(mano)
Reporter | ||
Comment 13•18 years ago
|
||
Comment on attachment 268055 [details] [diff] [review]
fix v2
r=mano.
Attachment #268055 -
Flags: review?(mano) → review+
Assignee | ||
Comment 14•18 years ago
|
||
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
Assignee | ||
Updated•18 years ago
|
Flags: in-testsuite?
Assignee | ||
Comment 15•18 years ago
|
||
Attachment #268208 -
Flags: review?(mano)
Assignee | ||
Updated•18 years ago
|
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 16•18 years ago
|
||
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+
Comment 17•17 years ago
|
||
Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a7pre) Gecko/200708020808 Minefield/3.0a7pre.
Status: RESOLVED → VERIFIED
Comment 18•15 years ago
|
||
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.
Description
•