Closed Bug 374613 Opened 15 years ago Closed 15 years ago

the context menu of places bookmarks with live titles is missing the "reload" option

Categories

(Firefox :: Bookmarks & History, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 3 alpha5

People

(Reporter: dietrich, Assigned: stevewon)

References

Details

Attachments

(1 file, 10 obsolete files)

In Fx2, microsummarized bookmarks have a context menu option for reloading the title, labeled "Reload Live Title". This needs to be added to the Places context menu for those bookmarks.
Blocks: 355737
Target Milestone: --- → Firefox 3 alpha5
Assignee: nobody → swon
Attached patch Proposing a patch (obsolete) — Splinter Review
Attachment #265874 - Flags: review?(dietrich)
Comment on attachment 265874 [details] [diff] [review]
Proposing a patch

This should be implemented as a separate command (and menu-item).
Attachment #265874 - Flags: review?(dietrich) → review-
Attached patch Proposed patch modified (obsolete) — Splinter Review
Attachment #265966 - Flags: review?(dietrich)
Comment on attachment 265966 [details] [diff] [review]
Proposed patch modified

seem to be missing the menu item in this patch.
Attachment #265966 - Flags: review?(dietrich)
This should have all the files. Sorry.
Attachment #265988 - Flags: review?(dietrich)
Comment on attachment 265988 [details] [diff] [review]
Patch with All other files missing from last one

the menu item in the organizer should only be enabled for microsummarized bookmarks, it's currently enabled for everything.

almost there :)
Attachment #265988 - Flags: review?(dietrich) → review-
Attached patch Weird (obsolete) — Splinter Review
So I fixed the problem with the edit menu under the Bookmark manager you mentioned. But now something funky happens. When you first right click on a bookmark with microsummary in the toolbar, "Reload Live Title" will be grayed out. But if you right click on it for the 2nd time, "Reload Live Title" will not be grayed out. So I tried dumping out the value for "this" (mentioned in the first couple +'s in this diff file), first time it prints out "null" and the second time its a valid object.
A little confused on this one..
(In reply to comment #7)
> Created an attachment (id=265999) [details]
> Weird
> 
> So I fixed the problem with the edit menu under the Bookmark manager you
> mentioned. But now something funky happens. When you first right click on a
> bookmark with microsummary in the toolbar, "Reload Live Title" will be grayed
> out. But if you right click on it for the 2nd time, "Reload Live Title" will
> not be grayed out. So I tried dumping out the value for "this" (mentioned in
> the first couple +'s in this diff file), first time it prints out "null" and
> the second time its a valid object.
> A little confused on this one..
> 

hm, i could not reproduce that problem. wfm on mac. what platform were you testing on?
A mac here also.. does everything work on your computer?
Make sure to add all the missing spaces before landing this... ;)
(In reply to comment #8)
...
> hm, i could not reproduce that problem. wfm on mac. what platform were you
> testing on?


That's bug 381477 which is basically a cocoa widget issue.
ah, it works fine when I right click. I kept ctrl+click before.
Sorry dietrich. Review please? =]
Attached patch Same patch as before. (obsolete) — Splinter Review
Attachment #265874 - Attachment is obsolete: true
Attachment #265966 - Attachment is obsolete: true
Attachment #265988 - Attachment is obsolete: true
Attachment #265999 - Attachment is obsolete: true
Attachment #266289 - Flags: review?(dietrich)
Attached patch Same patch as before. (obsolete) — Splinter Review
Attachment #266290 - Flags: review?(dietrich)
I will put up another one with spaces fixed.
Attached patch Patch with spacing fix (obsolete) — Splinter Review
Attachment #266289 - Attachment is obsolete: true
Attachment #266290 - Attachment is obsolete: true
Attachment #266295 - Flags: review?(dietrich)
Attachment #266289 - Flags: review?(dietrich)
Attachment #266290 - Flags: review?(dietrich)
Attachment #266295 - Attachment is obsolete: true
Attachment #266295 - Flags: review?(dietrich)
Target Milestone: Firefox 3 alpha5 → Firefox 3 alpha6
Attached patch Patch (obsolete) — Splinter Review
Attachment #266493 - Attachment is obsolete: true
Attachment #266781 - Flags: review?(mano)
Attached patch Patch (obsolete) — Splinter Review
Attachment #266781 - Attachment is obsolete: true
Attachment #266785 - Flags: review?(mano)
Attachment #266781 - Flags: review?(mano)
Comment on attachment 266785 [details] [diff] [review]
Patch

>Index: browser/components/places/content/controller.js
>===================================================================

>+    case "placesCmd_reloadMicrosummary":
>+      if (this._view.hasSingleSelection) {
>+        var selectedNode = this._view.selectedNode;
>+        var mss = PlacesUtils.microsummaries;
>+        if (mss.hasMicrosummary(selectedNode.itemId))

first check whether selectedNode is a bookmark node (PlacesUtils.nodeIsBookmark).

>Index: browser/components/places/content/places.xul
>===================================================================
>@@ -218,16 +218,19 @@
>           <menuitem command="placesCmd_setAsBookmarksToolbarFolder"
>                     label="&cmd.personalToolbarFolder.menuLabel;"
>                     accesskey="&cmd.personalToolbarFolder.menuAccesskey;"/>
>           <menuseparator/>
>           <menuitem id="editReload"
>                     command="placesCmd_reload"
>                     label="&cmd.reloadLivebookmark.label;"
>                     accesskey="&cmd.reloadLivebookmark.accesskey;"/>
>+          <menuitem id="editReloadMicrosummary"
>+                    command="placesCmd_reloadMicrosummary"
>+                    label="&cmd.reloadMicrosummary.label;"/>

This item used to have an accesskey in Fx2.

>Index: browser/components/places/content/placesOverlay.xul
>===================================================================
>   </commandset>
>@@ -174,16 +176,20 @@
>               selection="host|separator|link|folder|day"
>               forcehideselection="livemark/bookmarkFeedURI"/>
>     <menuseparator id="placesContext_deleteSeparator"/>
>     <menuitem id="placesContext_reload"
>               command="placesCmd_reload"
>               label="&cmd.reloadLivebookmark.label;"
>               accesskey="&cmd.reloadLivebookmark.accesskey;"
>               selection="livemark/feedURI|allLivemarks"/>
>+    <menuitem id="placesContext_reloadMicrosummary"
>+              command="placesCmd_reloadMicrosummary"
>+              label="&cmd.reloadMicrosummary.label;"

here too, probably.
Attachment #266785 - Flags: review?(mano) → review-
Attached patch PatchSplinter Review
Regarding the access key, "Reload Live Bookmark" and "Reload Live Bookmark" had the same access key in Fx2. So I set the access key in this patch as the access key for "Reload live Bookmark".
Attachment #266785 - Attachment is obsolete: true
Attachment #266801 - Flags: review?(mano)
mozilla/browser/components/places/content/controller.js 1.157
mozilla/browser/components/places/content/places.xul 1.75
mozilla/browser/components/places/content/placesOverlay.xul 1.12
mozilla/browser/components/places/content/utils.js 1.43
mozilla/browser/locales/en-US/chrome/browser/places/places.dtd 1.24
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: Firefox 3 alpha6 → Firefox 3 alpha5
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.