menuitems don't inherit the "hidden" attribute from commands

RESOLVED FIXED in mozilla19

Status

()

Core
XUL
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: paul, Assigned: Neil Deakin)

Tracking

(Blocks: 1 bug, {dev-doc-needed})

unspecified
mozilla19
dev-doc-needed
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

10.52 KB, patch
neil@parkwaycc.co.uk
: review+
smichaud
: review+
mixedpuppy
: review+
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
<command id="c1" hidden="true">
<broadcaster id="b1" hidden="true">

<toolbarbutton command="c1">
<menuitem1 observes="b1">
<menuitem2 command="c1">

toolbarbutton is hidden.
menuitem1 is hidden.
menuitem2 is not hidden.
Is this Mac-specific?
I'm seeing it on Windows as well.
OS: Mac OS X → All
Hardware: x86 → All
Neil: any ideas here?
(Assignee)

Comment 4

5 years ago
Created attachment 657318 [details] [diff] [review]
Patch

Well, one idea is to implement this for menuitems.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
(Assignee)

Updated

5 years ago
Attachment #657318 - Flags: review?(neil)

Comment 5

5 years ago
Comment on attachment 657318 [details] [diff] [review]
Patch

Seems reasonable to me, although I don't have a Mac to test that change.
Attachment #657318 - Flags: review?(neil) → review+

Updated

5 years ago
Keywords: dev-doc-needed
(Assignee)

Updated

5 years ago
Attachment #657318 - Flags: review?(smichaud)
Comment on attachment 657318 [details] [diff] [review]
Patch

Shane, can you test this out on Mac? It would be good to have so we can clean up http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-social.js#96
Attachment #657318 - Flags: review?(mixedpuppy)
I won't be able to get to this until (maybe) sometime next week.

So I'd be happy if Shane can test and review.
I'll take a look at it soon, busy building...
Comment on attachment 657318 [details] [diff] [review]
Patch

patch looks fine, tests pass, and a quick patch on socialapi shows that this allows us to resolve the issue jaws mentions in comment 6.
Attachment #657318 - Flags: review?(mixedpuppy) → review+
Comment on attachment 657318 [details] [diff] [review]
Patch

This looks fine to me.

I saw no problems testing it on OS X 10.7.5.

I right-clicked in a bunch of different contexts, and always got an appropriate result.  I also ran the included test manually a bunch of times, and saw no failures.
Attachment #657318 - Flags: review?(smichaud) → review+
https://hg.mozilla.org/mozilla-central/rev/9b7c3072b2a3
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
I'm having some trouble with this which I think is related to the fact that having a hidden attribute with *any* value acts as though hidden="true".  So the only way to have the command work as though it is not hidden is to remove the attribute from the command.  But if that is done, the patch doesn't update the hidden command on the menu item.

ie, there seems no way to remove the hidden attribute from the menuitem using just commands.
hrm - please ignore the previous comment - XUL elements do want explicitly hidden="true".  I'm not sure what problems I'm seeing but they don't seem to be related to this afaict.

Updated

5 years ago
Blocks: 805653

Updated

5 years ago
Depends on: 806781
Blocks: 808377
You need to log in before you can comment on or make changes to this bug.