Last Comment Bug 772808 - menuitems don't inherit the "hidden" attribute from commands
: menuitems don't inherit the "hidden" attribute from commands
: dev-doc-needed
Product: Core
Classification: Components
Component: XUL (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla19
Assigned To: Neil Deakin
: Neil Deakin
Depends on: 806781
Blocks: 805653 808377
  Show dependency treegraph
Reported: 2012-07-11 05:38 PDT by Paul Rouget [:paul]
Modified: 2012-11-05 17:53 PST (History)
6 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch (10.52 KB, patch)
2012-08-31 09:17 PDT, Neil Deakin
neil: review+
smichaud: review+
mixedpuppy: review+
Details | Diff | Splinter Review

Description User image Paul Rouget [:paul] 2012-07-11 05:38:26 PDT
<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.
Comment 1 User image :Gavin Sharp [email:] 2012-07-11 12:51:13 PDT
Is this Mac-specific?
Comment 2 User image Jared Wein [:jaws] (please needinfo? me) 2012-08-30 11:53:42 PDT
I'm seeing it on Windows as well.
Comment 3 User image :Gavin Sharp [email:] 2012-08-30 17:22:12 PDT
Neil: any ideas here?
Comment 4 User image Neil Deakin 2012-08-31 09:17:58 PDT
Created attachment 657318 [details] [diff] [review]

Well, one idea is to implement this for menuitems.
Comment 5 User image 2012-09-04 04:55:34 PDT
Comment on attachment 657318 [details] [diff] [review]

Seems reasonable to me, although I don't have a Mac to test that change.
Comment 6 User image Jared Wein [:jaws] (please needinfo? me) 2012-09-28 12:34:19 PDT
Comment on attachment 657318 [details] [diff] [review]

Shane, can you test this out on Mac? It would be good to have so we can clean up
Comment 7 User image Steven Michaud [:smichaud] (Retired) 2012-09-28 12:42:09 PDT
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.
Comment 8 User image Shane Caraveo (:mixedpuppy) 2012-09-28 15:28:23 PDT
I'll take a look at it soon, busy building...
Comment 9 User image Shane Caraveo (:mixedpuppy) 2012-09-28 16:09:38 PDT
Comment on attachment 657318 [details] [diff] [review]

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.
Comment 10 User image Steven Michaud [:smichaud] (Retired) 2012-10-08 14:03:33 PDT
Comment on attachment 657318 [details] [diff] [review]

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.
Comment 11 User image Ryan VanderMeulen [:RyanVM] 2012-10-19 19:03:59 PDT
Comment 12 User image Mark Hammond [:markh] 2012-10-22 22:20:54 PDT
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.
Comment 13 User image Mark Hammond [:markh] 2012-10-23 00:19:13 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.