Livemarks display regular bookmark icons on the sidebar

VERIFIED FIXED in Firefox 44

Status

()

defect
P2
normal
VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: avaida, Assigned: mak)

Tracking

({regression})

40 Branch
Firefox 44
Points:
---

Firefox Tracking Flags

(firefox39 unaffected, firefox40 affected, firefox44 verified)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Reporter

Description

4 years ago
Reproducible on:
* Beta 40.0b9 (20150730171029)

Affected platforms:
* Windows 10 Pro x64 (10240)
* Mac OS X 10.10.4
* Ubuntu 14.04 (x64)

Steps to reproduce:
1. Launch Firefox.
2. Subscribe to a few RSS feeds.
3. Enable the Bookmarks Sidebar and check the icons displayed for each of those feeds/livemarks.

Expected result:
Livemarks display their dedicated RSS icons in the sidebar.

Actual result:
A regular bookmark folder icon is displayed for the Livemarks in the sidebar.

Notes:
* This is NOT reproducible on Firefox 39.0 (20150630154324).
* If you're subscribing to an rss with the sidebar opened, the icon will be shown properly for it, but after a restart it will show the regular one instead - see  http://i.imgur.com/KJuLLLi.png.
* I'll follow up with a regression range as soon as possible.

Comment 1

4 years ago
Did/can you figure out what regressed this?
Flags: needinfo?(andrei.vaida)
Reporter

Comment 2

4 years ago
(In reply to :Gijs Kruitbosch from comment #1)
> Did/can you figure out what regressed this?

This is a really old regression, I've managed to track it back to Firefox 23 nightly builds from 2013-05-04, I think that at this point, going further with the bisection wouldn't provide any useful data.
Flags: needinfo?(andrei.vaida)

Comment 3

4 years ago
(In reply to Andrei Vaida, QA [:avaida] from comment #2)
> (In reply to :Gijs Kruitbosch from comment #1)
> > Did/can you figure out what regressed this?
> 
> This is a really old regression, I've managed to track it back to Firefox 23
> nightly builds from 2013-05-04, I think that at this point, going further
> with the bisection wouldn't provide any useful data.

This doesn't make sense, comment #0 says it doesn't reproduce with Firefox 39. Which is it?
Flags: needinfo?(andrei.vaida)
Reporter

Comment 4

4 years ago
(In reply to :Gijs Kruitbosch from comment #3)
> This doesn't make sense, comment #0 says it doesn't reproduce with Firefox
> 39. Which is it?

What I meant to say is that after a more thorough investigation of this issue, it turns it's reproducible all the way back to Firefox 23, including Firefox 39 :). So Comment 2 stands.
Flags: needinfo?(andrei.vaida)

Comment 5

4 years ago
diff --git a/browser/components/places/content/treeView.js b/browser/components/places/content/treeView.js
--- a/browser/components/places/content/treeView.js
+++ b/browser/components/places/content/treeView.js
@@ -1160,17 +1160,17 @@ PlacesTreeView.prototype = {
                  nodeType == Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT) {
           if (this._controller.hasCachedLivemarkInfo(node)) {
             properties += " livemark";
           }
           else {
             PlacesUtils.livemarks.getLivemark({ id: node.itemId })
               .then(aLivemark => {
                 this._controller.cacheLivemarkInfo(node, aLivemark);
-                properties += " livemark";
+                this._cellProperties.set(node, undefined);
                 // The livemark attribute is set as a cell property on the title cell.
                 this._invalidateCellValue(node, this.COLUMN_TYPE_TITLE);
               }, () => undefined);
           }
         }
 
         if (itemId != -1) {
           let queryName = PlacesUIUtils.getLeftPaneQueryNameFromId(itemId);

Comment 6

4 years ago
(In reply to atlanto from comment #5)
> diff --git a/browser/components/places/content/treeView.js
> b/browser/components/places/content/treeView.js
> --- a/browser/components/places/content/treeView.js
> +++ b/browser/components/places/content/treeView.js
> @@ -1160,17 +1160,17 @@ PlacesTreeView.prototype = {
>                   nodeType ==
> Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT) {
>            if (this._controller.hasCachedLivemarkInfo(node)) {
>              properties += " livemark";
>            }
>            else {
>              PlacesUtils.livemarks.getLivemark({ id: node.itemId })
>                .then(aLivemark => {
>                  this._controller.cacheLivemarkInfo(node, aLivemark);
> -                properties += " livemark";
> +                this._cellProperties.set(node, undefined);
>                  // The livemark attribute is set as a cell property on the
> title cell.
>                  this._invalidateCellValue(node, this.COLUMN_TYPE_TITLE);
>                }, () => undefined);
>            }
>          }
>  
>          if (itemId != -1) {
>            let queryName = PlacesUIUtils.getLeftPaneQueryNameFromId(itemId);

I don't know enough about places to figure out if this would fix the issue and why (and if the other properties += " livemark" also needs updating ?)
Flags: needinfo?(mak77)
Assignee

Comment 7

4 years ago
the proposed change is not exactly right, but it hits the nail on the head.
Assignee: nobody → mak77
Flags: needinfo?(mak77)
Assignee

Updated

4 years ago
Priority: -- → P2
Assignee

Comment 8

4 years ago
Posted patch patch v1 (obsolete) — Splinter Review
sorry for multiple review requests today, it's trivial stuff though (and I'm done with this for now)
Attachment #8673724 - Flags: review?(gijskruitbosch+bugs)
Assignee

Comment 9

4 years ago
Posted patch patch v1.1Splinter Review
stupid copy paste typo.
Attachment #8673724 - Attachment is obsolete: true
Attachment #8673724 - Flags: review?(gijskruitbosch+bugs)
Assignee

Updated

4 years ago
Attachment #8673726 - Flags: review?(gijskruitbosch+bugs)

Updated

4 years ago
Attachment #8673726 - Flags: review?(gijskruitbosch+bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/a7eb74116e1b
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Reporter

Updated

4 years ago
QA Whiteboard: [good first verify]
I have reproduced this bug with Firefox Nightly 42.0a1 (Build: 20150803030207)on 
windows 8.1 pro 64-bit with the instructions from comment 0 .

Verified as fixed with Latest Firefox Beta 44.0b7 (Build ID: 20160107144911)
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0
QA Whiteboard: [good first verify] → [good first verify][testday-20160108]

Updated

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