Closed Bug 371798 Opened 16 years ago Closed 16 years ago

query result nodes must update titles on bookmark id not URI

Categories

(Firefox :: Bookmarks & History, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dietrich, Assigned: dietrich)

References

Details

Attachments

(3 files, 1 obsolete file)

From https://bugzilla.mozilla.org/show_bug.cgi?id=360133#c15:

> Now I'm searching for child nodes by bookmark id instead of URI
> where appropriate.

Yes, that's exactly what I'd like to say! And finally we got ready to
fight against another URI-base enemy "ChangeTitles(...)".

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/components/places/src/nsNavHistoryResult.cpp&rev=1.79&mark=2868-2874#2868
Blocks: 370099
Assignee: nobody → dietrich
Comment on attachment 258271 [details] [diff] [review]
fix v1 - for folder nodes, update titles by bookmark id instead of uri

r=sspitzer
Attachment #258271 - Flags: review?(sspitzer) → review+
do we have a test for this?
Flags: in-testsuite?
Attached patch test v1 (obsolete) — Splinter Review
wip test. haven't run it yet, as mochitest isn't running the tests in browser/components/places/tests/chrome.

rob, there's a test in there already, but in the mochitest output, the dirs are shown, but no tests run. any idea why?
(In reply to comment #4)
> Created an attachment (id=258333) [details]
> test v1
> 
> wip test. haven't run it yet, as mochitest isn't running the tests in
> browser/components/places/tests/chrome.

perl runtests.pl --chrome

> 
> rob, there's a test in there already, but in the mochitest output, the dirs are
> shown, but no tests run. any idea why?

It copies non-test resources for the add_livemark test onto the http server's web root. As there are more tests added to the directory, it should make more sense.

Comment on attachment 258333 [details] [diff] [review]
test v1


>+var toolbar = document.getElementById("bookmarksBarContent");

rob, any idea why i wouldn't be able to access nodes in chrome? the above statement works fine in core and extensions, but not in mochitest.
(In reply to comment #6)
> (From update of attachment 258333 [details] [diff] [review])
> 
> >+var toolbar = document.getElementById("bookmarksBarContent");
> 
> rob, any idea why i wouldn't be able to access nodes in chrome? the above
> statement works fine in core and extensions, but not in mochitest.

That code is looking for the toolbar inside the content document--the one that corresponds to the XUL test file. You need to grab the top level window and ask for its XUL doc.

(In reply to comment #7)
> (In reply to comment #6)
> > (From update of attachment 258333 [details] [diff] [review] [details])
> > 
> > >+var toolbar = document.getElementById("bookmarksBarContent");
> > 
> > rob, any idea why i wouldn't be able to access nodes in chrome? the above
> > statement works fine in core and extensions, but not in mochitest.
> 
> That code is looking for the toolbar inside the content document--the one that
> corresponds to the XUL test file. You need to grab the top level window and ask
> for its XUL doc.
> 

ah, ok i see. it'd be great if there was an option to run the chrome test as an overlay, or something that would allow it to run in the same environment as core browser code.
Please do not make API tests depend on the browser UI.
hm, yeah i guess it's more of an end-to-end test. not sure how i'd isolate this change in a test.
Attached patch test v2Splinter Review
Attachment #258333 - Attachment is obsolete: true
Attachment #259964 - Flags: review?(sayrer)
Comment on attachment 259964 [details] [diff] [review]
test v2

looks good to me
Attachment #259964 - Flags: review?(sayrer) → review+
Checking in toolkit/components/places/tests/chrome/Makefile.in;
/cvsroot/mozilla/toolkit/components/places/tests/chrome/Makefile.in,v  <--  Makefile.in
new revision: 1.3; previous revision: 1.2
done
RCS file: /cvsroot/mozilla/toolkit/components/places/tests/chrome/test_371798.xul,v
done
Checking in toolkit/components/places/tests/chrome/test_371798.xul;
/cvsroot/mozilla/toolkit/components/places/tests/chrome/test_371798.xul,v  <--  test_371798.xul
initial revision: 1.1
done
Status: NEW → RESOLVED
Closed: 16 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Attached patch test v3Splinter Review
race condition in the test (test can execute before the query result has been live-updated).

this makes the test run after the bookmarks service has sent out notifications.
Attachment #260372 - Flags: review?(sayrer)
Attachment #260372 - Flags: review?(sayrer) → review+
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.