Closed
Bug 370215
Opened 18 years ago
Closed 18 years ago
update microsummaries to use unique place: uris for Places bookmarks
Categories
(Firefox Graveyard :: Microsummaries, defect, P2)
Firefox Graveyard
Microsummaries
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 3 alpha3
People
(Reporter: myk, Assigned: asaf)
References
Details
Attachments
(1 file)
15.93 KB,
patch
|
myk
:
review+
dietrich
:
review+
|
Details | Diff | Splinter Review |
In bug 360133, bookmarks in Places are getting unique identifiers. The microsummaries code needs to be updated to use those identifiers instead of URIs to refer to the bookmarks it summarizes.
Reporter | ||
Comment 1•18 years ago
|
||
Among other things, this means fixing references in nsMicrosummaryService.js to bookmarkID.spec so that they refer to the PRInt64 equivalent (probably just bookmarkID).
Assignee | ||
Comment 2•18 years ago
|
||
Assignee: nobody → mano
Priority: -- → P2
Target Milestone: --- → Firefox 3 alpha3
Assignee | ||
Comment 3•18 years ago
|
||
Attachment #258160 -
Flags: review?(myk)
Assignee | ||
Updated•18 years ago
|
Attachment #258160 -
Flags: review?(dietrich)
Reporter | ||
Comment 4•18 years ago
|
||
Comment on attachment 258160 [details] [diff] [review]
patch
>- _getField: function MSS__getField(bookmarkID, fieldName) {
>- var pageURI = bookmarkID.QueryInterface(Ci.nsIURI);
>+ _getField: function MSS__getField(aPlaceURI, aFieldName) {
>+ aPlaceURI.QueryInterface(Ci.nsIURI);
Nit: if I recall correctly, mconnor has said we are standardizing on not having "a" prefixes on method arguments, so these should be prefix-less here and elsewhere.
Otherwise, this looks fine. I haven't been able to test, however, as Places is horked on my build.
Attachment #258160 -
Flags: review?(myk) → review+
Comment 5•18 years ago
|
||
Myk, where did mconnor say (or write) about not using Hungarian notation? (I'm sure you're going to get a bunch of people who like it, and a bunch of people who don't.)
Reporter | ||
Comment 6•18 years ago
|
||
(In reply to comment #5)
> Myk, where did mconnor say (or write) about not using Hungarian notation?
I don't recall, but I'll ping him shortly to confirm. Perhaps I misunderstood or misattributed that statement to mconnor when it was actually uttered by someone less authoritative.
> (I'm sure you're going to get a bunch of people who like it, and a bunch of
> people who don't.)
Right. We'll never make everyone happy no matter which way we go on this.
Reporter | ||
Comment 7•18 years ago
|
||
(In reply to comment #6)
> (In reply to comment #5)
> > Myk, where did mconnor say (or write) about not using Hungarian notation?
>
> I don't recall, but I'll ping him shortly to confirm. Perhaps I misunderstood
> or misattributed that statement to mconnor when it was actually uttered by
> someone less authoritative.
Correction: no-a was Ben's position; yes-a was mconnor's, and we're going with yes-a. So the review stands with the nit rescinded.
Updated•18 years ago
|
Attachment #258160 -
Flags: review?(dietrich) → review+
Assignee | ||
Updated•18 years ago
|
Summary: update microsummaries to use new unique identifiers for Places bookmarks → update microsummaries to use unique place: uris for Places bookmarks
Assignee | ||
Comment 8•18 years ago
|
||
mozilla/browser/components/microsummaries/src/nsMicrosummaryService.js 1.58
mozilla/browser/components/places/content/bookmarkProperties.js 1.35
mozilla/browser/components/places/content/controller.js 1.136
mozilla/browser/components/places/content/toolbar.xml 1.76
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Updated•9 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•