[MacOSX] Sub-menus in native Bookmarks menu have "Empty" menuitem

RESOLVED FIXED in seamonkey2.1a1

Status

SeaMonkey
Bookmarks & History
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: stefanh, Assigned: stefanh)

Tracking

({fixed-seamonkey2.0.1, pp, regression})

Trunk
seamonkey2.1a1
x86
Mac OS X
fixed-seamonkey2.0.1, pp, regression

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

8 years ago
The "Empty" menuitem is hidden with css - this doesn't work in mac native menus.
Flags: blocking-seamonkey2.0.1?
(Assignee)

Updated

8 years ago
Version: SeaMonkey 2.0 Branch → Trunk
Keywords: pp
Summary: Sub-menus in native Bookmarks menu has "Empty" menuitem → [MacOSX] Sub-menus in native Bookmarks menu have "Empty" menuitem
(Assignee)

Comment 1

8 years ago
I think it should work if we just play with the hidden attribute when the popupshowing event occurs. It should also be enough to do this for the main menu only.
(Assignee)

Comment 2

8 years ago
Created attachment 411011 [details] [diff] [review]
needs testing

This patch works for me. However, I discovered some strange behaviour:

1) Create an empty folder in the PT.
2) Open the (XUL) bm menu in the the PT and move (d & d) some bookmarks into the newly created folder
3) Open the native bm menu, look for the newly added bookmarks - they're there
4) Open the bm menu in the PT, drag & drop one bm out of the folder
5) Open the native bm menu

--> the folder still contains 2 bookmarks

I can't see how this would be related to the patch, though - need some further investigation.

Comment 3

8 years ago
I see the following with the patch applied on Linux:
1) Create an empty folder in bookmarks menu
2) Drag and drop link onto empty folder

Expected result
1) Link replaces "empty" entry

Actual result
1) Link shows below "empty" entry

When you re-show the bookmarks, "empty" entry has gone.
(Assignee)

Comment 4

8 years ago
Hmm, maybe I should keep the css.
(Assignee)

Comment 5

8 years ago
Created attachment 412414 [details] [diff] [review]
Keep class name and stop the event bubbling

This should address comment #3, asking Ian to get Linux confirmation. Regarding my comment #2, I managed to reproduce this in 2.0 so it doesn't look related to my patch. I guess the native menu sometimes doesn't re-build (or something like that) properly.
Assignee: nobody → stefanh
Attachment #411011 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #412414 - Flags: superreview?(neil)
Attachment #412414 - Flags: review?(mnyromyr)
Attachment #412414 - Flags: review?(iann_bugzilla)

Comment 6

8 years ago
Comment on attachment 412414 [details] [diff] [review]
Keep class name and stop the event bubbling

You might want to restore the comment (but change "will" to "may").

>+                     onpopupshowing="this.firstChild.hidden = (this.childNodes.length > 1); event.stopPropagation();">
Nit: doesn't need ()s around the boolean expression.
Attachment #412414 - Flags: superreview?(neil) → superreview+
(Assignee)

Comment 7

8 years ago
(In reply to comment #6)
> (From update of attachment 412414 [details] [diff] [review])
> You might want to restore the comment (but change "will" to "may").

Ah, right - will do.
> 
> >+                     onpopupshowing="this.firstChild.hidden = (this.childNodes.length > 1); event.stopPropagation();">
> Nit: doesn't need ()s around the boolean expression.
OK.

Comment 8

8 years ago
Comment on attachment 412414 [details] [diff] [review]
Keep class name and stop the event bubbling

Works fine on linux r=me
Attachment #412414 - Flags: review?(iann_bugzilla) → review+

Comment 9

8 years ago
Comment on attachment 412414 [details] [diff] [review]
Keep class name and stop the event bubbling

r=me for the Mac side.
Attachment #412414 - Flags: review?(mnyromyr) → review+
(Assignee)

Comment 10

8 years ago
http://hg.mozilla.org/comm-central/rev/ee7d93c37a69
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1a1
(Assignee)

Comment 11

8 years ago
Created attachment 414912 [details] [diff] [review]
What I landed

I think we need this for 2.0.1 - it's a safe fix that fixes a highly visible problem on mac.
Attachment #414912 - Flags: approval-seamonkey2.0.1?

Updated

8 years ago
Attachment #414912 - Flags: approval-seamonkey2.0.1? → approval-seamonkey2.0.1+
(Assignee)

Comment 12

8 years ago
http://hg.mozilla.org/releases/comm-1.9.1/rev/1f90b2bf7dc7
Keywords: fixed-seamonkey2.0.1
(Assignee)

Updated

8 years ago
Flags: blocking-seamonkey2.0.1?
You need to log in before you can comment on or make changes to this bug.