"Subscribe to This Page" in Bookmarks button menu always disabled and lacks icon

VERIFIED FIXED in seamonkey2.1b3

Status

SeaMonkey
Feed Discovery and Preview
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: stefanh, Assigned: neil@parkwaycc.co.uk)

Tracking

Trunk
seamonkey2.1b3
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
Recent trunk, all OS (Callek confirmed Windows)

STR:

1) Load planet.m.o and open the Bookmarks menu in the Bookmarks toolbar (note that it's the BM toolbar menu)
2) Look at the "Subscribe to This Page" menu

Expected result: Enabled menu with a feeds icon
Actual result: Disabled menu without feeds icon
(Reporter)

Comment 1

6 years ago
I think this should block, having a menitem that doesn't work seems to me like really bad ui. We should either remove it (it's not there in 2.0.x) or make it work.
blocking-seamonkey2.1: --- → ?
Confirmed (again) on Windows.

SM 2.0 and 2.1:
window Bookmarks menu has this menuitem;
menuitem has the "feeds" icon;
menuitem is enabled on http://planet.mozilla.org/ page.
The feed icon in the Location Bar works too.

SM 2.1 only:
personal toolbar Bookmarks menu has this menuitem;
and this bug.

http://mxr.mozilla.org/comm-central/search?string=populateFeeds&case=1&find=%2Fsuite%2F
The 3 occurrences

***

http://mxr.mozilla.org/comm-central/search?string=feedsMenu&case=1&find=%2Fsuite%2F

Iiuc, nsBrowserStatusHandler.js handles (the) 1 'id=feedsMenu' only :-|
(In reply to comment #2)
> http://mxr.mozilla.org/comm-central/search?string=feedsMenu&case=1&find=%2Fsuite%2F
> 
> (the) 1 'id=feedsMenu' only :-|

And, because of the same lack of id, css rules don't apply the icon there :-|
(In reply to comment #2)
> SM 2.1 only:
> personal toolbar Bookmarks menu has this menuitem;
> and this bug.

Ftr, adding this occurrence is blamed to bug 580660.
Depends on: 580660
(Reporter)

Comment 5

6 years ago
Maybe having a BMB_feedsmenu id and then fix nsBrowserStatusHandler.js to handle it would be ok.
(Reporter)

Comment 6

6 years ago
Created attachment 520554 [details] [diff] [review]
Proposed patch

I went with 'bmbFeedsMenu' since that fits better with the style in the js file.
Assignee: nobody → stefanh
Status: NEW → ASSIGNED
Attachment #520554 - Flags: review?(neil)
(Reporter)

Comment 7

6 years ago
Comment on attachment 520554 [details] [diff] [review]
Proposed patch

Forgot to say that the "!important" didn't seemed necessary, but it's not tested on non-mac.
(Reporter)

Comment 8

6 years ago
I will file a follow-up, since on Mac, I need an icon for the disabled state (the native menu handles this for free, so I don't have a ready one and the windows one is most likely too washed-out).
(Reporter)

Updated

6 years ago
Blocks: 643323
(Assignee)

Comment 9

6 years ago
Created attachment 520557 [details] [diff] [review]
Alternative approach
[Checked in: See comment 12]
Attachment #520557 - Flags: review?(stefanh)
Comment on attachment 520557 [details] [diff] [review]
Alternative approach
[Checked in: See comment 12]

Just what I was hoping for: class and command ;-)

Should '!important' be removed from the 2 non-Mac files too?
Attachment #520557 - Flags: feedback+
(Reporter)

Comment 11

6 years ago
Comment on attachment 520557 [details] [diff] [review]
Alternative approach
[Checked in: See comment 12]

This is much better, thanks :-) Should we have separate names for class and id?
Attachment #520557 - Flags: review?(stefanh) → review+
(Reporter)

Updated

6 years ago
Assignee: stefanh → neil
Target Milestone: --- → seamonkey2.1b3
(Reporter)

Updated

6 years ago
Attachment #520554 - Flags: review?(neil)
Attachment #520554 - Attachment is obsolete: true
(Assignee)

Comment 12

6 years ago
Pushed changeset 73a13310cbed to comm-central, removing all the !important
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Summary: "Subscribe to This Page" in Bookmarks toolbar menu always disabled and lacks icon → "Subscribe to This Page" in Bookmarks button menu always disabled and lacks icon
blocking-seamonkey2.1: ? → ---
Attachment #520557 - Attachment description: Alternative approach → Alternative approach [Checked in: See comment 12]
[Mozilla/5.0 (Windows NT 5.0; rv:2.2a1pre) Gecko/20110323 Firefox/4.0b13pre SeaMonkey/2.1b3pre] (comm-central-trunk-win32/...)

Icon is there and item activates. (at the 3 places)

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