Closed Bug 912855 Opened 6 years ago Closed 6 years ago

Can't remove check-mark on "Load this bookmark in the sidebar" in Firefox 25 and 26

Categories

(Toolkit :: Places, defect)

25 Branch
x86
All
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla27
Tracking Status
firefox24 --- unaffected
firefox25 + verified
firefox26 + verified
firefox27 --- verified

People

(Reporter: bugdickvl, Assigned: mano)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

It is possible to set a check-mark on "Load this bookmark in the sidebar" in the Library, but it isn't possible to remove this check-mark.

A check of places.sqlite with the SQLite Manager shows that a bookmarkProperties/loadInSidebar entry in moz_items_annos is created with a value of 1.

When I try to remove the check-mark then the value is changed to 0 and the check-box is still checked (pressing the space bar gives a short off/on blink).

This still works correctly in the current Firefox 24 beta version, but doesn't work in Aurora (25a2) and Nightly (26a1) builds.
Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/5197eef777dd
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0 ID:20130803010030
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/714a27789e9e
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0 ID:20130803040718
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5197eef777dd&tochange=714a27789e9e
Regressed by 
714a27789e9e	Asaf Romano — Bug 895839 - Remove support for binary annotations. r=mak. sr=gavin.
Blocks: 895839
Status: UNCONFIRMED → NEW
Component: Bookmarks & History → Places
Ever confirmed: true
Keywords: regression
Product: Firefox → Toolkit
yes, this is an unexpected regression
Flags: needinfo?(mano)
Attached patch patch (obsolete) — Splinter Review
Attachment #802411 - Flags: review?(mak77)
Flags: needinfo?(mano)
Comment on attachment 802411 [details] [diff] [review]
patch

Review of attachment 802411 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/places/content/editBookmarkOverlay.js
@@ +589,5 @@
>  
>    onLoadInSidebarCheckboxCommand:
>    function EIO_onLoadInSidebarCheckboxCommand() {
>      var loadInSidebarChecked = this._element("loadInSidebarCheckbox").checked;
> +    var annoObj = { name   : PlacesUIUtils.LOAD_IN_SIDEBAR_ANNO };

please fix indentation before ":", now that there's only one property

@@ +590,5 @@
>    onLoadInSidebarCheckboxCommand:
>    function EIO_onLoadInSidebarCheckboxCommand() {
>      var loadInSidebarChecked = this._element("loadInSidebarCheckbox").checked;
> +    var annoObj = { name   : PlacesUIUtils.LOAD_IN_SIDEBAR_ANNO };
> +    if (this._element("loadInSidebarCheckbox").checked)

you already have loadInSidebarChecked... maybe you wanted to remove it?

@@ +592,5 @@
>      var loadInSidebarChecked = this._element("loadInSidebarCheckbox").checked;
> +    var annoObj = { name   : PlacesUIUtils.LOAD_IN_SIDEBAR_ANNO };
> +    if (this._element("loadInSidebarCheckbox").checked)
> +      annoObj.value = true;
> +    var txn = new PlacesSetItemAnnotationTransaction(this._itemId, annoObj);

while here, s/var/let/
Attachment #802411 - Flags: review?(mak77) → review+
Ready to land?
Flags: needinfo?(mano)
Attached patch patch v2Splinter Review
updated patch addressing review comments
Attachment #802411 - Attachment is obsolete: true
Flags: needinfo?(mano)
https://hg.mozilla.org/integration/fx-team/rev/b088976006ef
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla27
Comment on attachment 814900 [details] [diff] [review]
patch v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 895839
User impact if declined: Load in sidebar options can't be changed in the ui
Testing completed (on m-c, etc.): local testing
Risk to taking this patch (and alternatives if risky): low, trivial patch
String or IDL/UUID changes made by this patch: none
Attachment #814900 - Flags: approval-mozilla-beta?
Attachment #814900 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/b088976006ef
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Attachment #814900 - Flags: approval-mozilla-beta?
Attachment #814900 - Flags: approval-mozilla-beta+
Attachment #814900 - Flags: approval-mozilla-aurora?
Attachment #814900 - Flags: approval-mozilla-aurora+
Keywords: verifyme
Mozilla/5.0 (Windows NT 6.1; rv:25.0) Gecko/20100101 Firefox/25.0
Mozilla/5.0 (X11; Linux i686; rv:25.0) Gecko/20100101 Firefox/25.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:25.0) Gecko/20100101 Firefox/25.0

Verified as fixed on Firefox 25 beta 7 (Build ID: 20131010180222), the "Load this bookmark in the sidebar" check-mark can be unchecked.
QA Contact: cornel.ionce
Verified as fixed on latest Aurora (20131016004005):

Mozilla/5.0 (X11; Linux i686; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (Windows NT 6.2; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (X11; Linux x86_64; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Windows NT 6.1; rv:27.0) Gecko/20100101 Firefox/27.0

Verified as fixed on latest Aurora 27.0a2 (Build ID: 20131104004000) and latest Nightly 28.0a1 (Build ID: 20131104044747).
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.