Add "Show editor when saving" checkbox to the Bookmark panel

VERIFIED FIXED in Firefox 63

Status

()

enhancement
P2
normal
VERIFIED FIXED
Last year
Last year

People

(Reporter: dao, Assigned: dao)

Tracking

(Blocks 1 bug)

Trunk
Firefox 63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 verified)

Details

(Whiteboard: [fxsearch])

Attachments

(2 attachments)

Assignee

Description

Last year
Posted image Mockup
When unchecked, this will disable the dialog for the next time and show a "Saved to Bookmarks!" confirmation instead.

The checkbox should also be present when editing an existing bookmark, such that users can re-check it if they want.
Assignee

Updated

Last year
Priority: -- → P1
Assignee

Updated

Last year
Priority: P1 → P2
Assignee

Updated

Last year
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Assignee

Updated

Last year
Depends on: 1471580
Assignee

Updated

Last year
Depends on: 1471585
Assignee

Updated

Last year
Summary: Add "Show editor when saving" checkbox to the Bookmark dialog → Add "Show editor when saving" checkbox to the Bookmark panel
Whiteboard: [fxsearch]

Comment 3

Last year
mozreview-review
Comment on attachment 8988316 [details]
Bug 1459878 - Add "Show editor when saving" checkbox to the Bookmark panel.

https://reviewboard.mozilla.org/r/253576/#review260386

There is one thing a bit underspecified, that is whether we want the same behavior for mouse and keyboard interactions. For example in the past we used to not open the panel with the mouse but open it with the keyboard. But I think in general we are moving towards more coherence between those, so I assume it is ok to handle both through this checkbox.

::: browser/app/profile/firefox.js:989
(Diff revision 1)
>  pref("browser.security.newcerterrorpage.enabled", false);
>  
>  // Whether to start the private browsing mode at application startup
>  pref("browser.privatebrowsing.autostart", false);
>  
> +pref("browser.bookmarks.editDialog.showForNewBookmarks", true);

please add a brief comment

::: browser/base/content/browser-places.js:239
(Diff revision 1)
>          gNavigatorBundle.getString("editBookmarkPanel.newBookmarkTitle") :
>          gNavigatorBundle.getString("editBookmarkPanel.editBookmarkTitle");
>  
> -    // No description; show the Done, Remove;
> +    let showForNewBookmarksCheckbox = this._element("editBMPanel_showForNewBookmarks");
> +    showForNewBookmarksCheckbox.checked = this.showForNewBookmarks;
> +    showForNewBookmarksCheckbox.hidden = false;

is it hidden by default for perf reasons? does it actually matter, since the whole panel is hidden?

Actually I think editBookmarkPanelBottomButtons and editBookmarkPanelContent are hidden just for not showing stale info in case of problems, but this checkbox refers to the panel, more than the bookmark, so it probably doesn't need to be hidden.

::: browser/components/places/content/editBookmarkPanel.inc.xul:126
(Diff revision 1)
>        <textbox id="editBMPanel_keywordField"
>                 onchange="gEditItemOverlay.onKeywordFieldChange();"/>
>      </vbox>
>    </vbox>
>  
> +  <checkbox id="editBMPanel_showForNewBookmarks"

I don't think this should be in this overlay, Is there a specific reason to not put it directly into "editBookmarkPanel" in browser.xul?

The overlay here is about stuff shared across all the bookmark panels, while this is specific to the star panel

I uderstand there may be additional aligning needed for that, but I'd still take that hit for better code separation.

::: browser/components/places/content/editBookmarkPanel.inc.xul:130
(Diff revision 1)
>  
> +  <checkbox id="editBMPanel_showForNewBookmarks"
> +            hidden="true"
> +            label="&editBookmarkOverlay.showForNewBookmarks.label;"
> +            accesskey="&editBookmarkOverlay.showForNewBookmarks.accesskey;"
> +            oncommand="gEditItemOverlay.onShowForNewBookmarksCheckboxCommand();"/>

same way the handles should probably be in StarUI in browser-places.js

::: browser/locales/en-US/chrome/browser/places/editBookmarkOverlay.dtd:26
(Diff revision 1)
>  <!ENTITY editBookmarkOverlay.tagsExpanderDown.tooltip        "Show all tags">
>  <!ENTITY editBookmarkOverlay.choose.label                    "Choose…">
>  <!ENTITY editBookmarkOverlay.newFolderButton.label           "New Folder">
>  <!ENTITY editBookmarkOverlay.newFolderButton.accesskey       "o">
> +<!ENTITY editBookmarkOverlay.showForNewBookmarks.label       "Show editor when saving">
> +<!ENTITY editBookmarkOverlay.showForNewBookmarks.accesskey   "S">

and the labels in browser.dtd
Attachment #8988316 - Flags: review?(mak77)
Comment hidden (mozreview-request)

Comment 5

Last year
mozreview-review
Comment on attachment 8988316 [details]
Bug 1459878 - Add "Show editor when saving" checkbox to the Bookmark panel.

https://reviewboard.mozilla.org/r/253576/#review260708
Attachment #8988316 - Flags: review?(mak77) → review+

Comment 6

Last year
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7666316c4222
Add "Show editor when saving" checkbox to the Bookmark panel. r=mak
Assignee

Updated

Last year
Blocks: 1472275

Comment 7

Last year
bugherder
https://hg.mozilla.org/mozilla-central/rev/7666316c4222
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
I have reproduced this bug with Nightly 62.0a1 (2018-05-08) on Windows 10, 64 Bit!
This bug's fix is verified with latest Nightly!

Build ID 	20180704100142
User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0
QA Whiteboard: [bugday-20180704]
Assignee

Updated

Last year
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.