Open
Bug 309953
Opened 20 years ago
Updated 3 years ago
toolbarbuttons aren't in sync with their commands after customizing
Categories
(Core :: XUL, defect)
Tracking
()
NEW
People
(Reporter: asaf, Unassigned, NeedInfo)
References
Details
Attachments
(3 files)
2.84 KB,
application/vnd.mozilla.xul+xml
|
Details | |
2.53 KB,
patch
|
Details | Diff | Splinter Review | |
2.61 KB,
patch
|
asaf
:
review+
|
Details | Diff | Splinter Review |
When a toolbarbutton is wrapped for toolbars customizing, we save its command
attribute as an "itemcommand" attribute, on the wraper; we set it back (as
"command") when the toolbarbutton is unwraped. However, at this point, the
command attributes (e.g. "disabled") aren't in sync with the toolbarbutton.
See also bug 287105.
Reporter | ||
Updated•20 years ago
|
Assignee: nobody → general
Component: Toolbars and Toolbar Customization → XBL
Flags: blocking1.9a1?
Product: Toolkit → Core
QA Contact: nobody → ian
Version: unspecified → 1.8 Branch
![]() |
||
Comment 1•20 years ago
|
||
So what does this have to do with XBL, exactly? That is, what is the bug in
content/xbl/src that needs fixing? I see neither bug description nor steps to
reproduce.
Reporter | ||
Comment 2•20 years ago
|
||
I might be wrong in the component, although i recall we are inheriting the
attributes from the command element (based on the command attribute) in contnet/xbl.
If firefox can be considered a test case, the steps described in bug 287105
comment 0 raise this bug.
![]() |
||
Comment 3•20 years ago
|
||
> although i recall we are inheriting the attributes from the command element
What command element?
And no, Firefox is not a testcase. The toolbar customization dialog is a huge
hack that does all sorts of weird (and some unsupported) stuff; just because it
has an issue doesn't mean it's not cauing the issue itself.
If someone ever provides something resembling a real bug description here, I'll
look at this at that point.
Keywords: qawanted
Reporter | ||
Updated•20 years ago
|
Assignee: general → nobody
Component: XBL → Toolbars and Toolbar Customization
Product: Core → Toolkit
QA Contact: ian → nobody
Reporter | ||
Updated•20 years ago
|
Flags: blocking1.9a1?
Comment 4•18 years ago
|
||
Testcase:
One thing is certain: when the initial state of a toolbar button is disabled, and it is not one of the default buttons, the button is saved in the toolbox's palette with a disabled state.
However with the reload button: its state is most of the time not disabled. But when you remove it from the toolbar its state is remembered as disabled.
But that is not the real issue here.
The issue is that when you drag a disabled toolbar button to the toolbar its state does not reflect its commands state. This is what the testcase shows.
But after this whenever the commands state is updated, the state of the toolbar button will automatically be update to, see bug 287105 comment 0 (add new tab: button is enabled)
Comment 5•18 years ago
|
||
This will fix the state of newly added toolbar buttons reflecting the state of their commands.
Comment 6•18 years ago
|
||
Asking for blocking 1.9, because (most) extensions don't have their buttons on a toolbar by default. And when for instance the state of a command is enabled once and never disabled again in a single browser session, a newly added button can't be enabled until Firefox is restarted (bad first impression for an extension).
Flags: blocking1.9?
Comment 7•18 years ago
|
||
not a blocker, would take a reviewed patch before b2
Flags: wanted1.9+
Flags: blocking1.9?
Flags: blocking1.9-
Updated•18 years ago
|
QA Contact: nobody → toolbars
Reporter | ||
Comment 8•18 years ago
|
||
Comment on attachment 287618 [details] [diff] [review]
simple patch
This ought to be fixed in core imo, or we should fix our way of setting the command on the toolbar-element, I didn't read this code for a long while.
Attachment #287618 -
Flags: review?(mano)
Updated•17 years ago
|
Comment 9•17 years ago
|
||
Bug 440702 is adding a few more instances of the hack from bug 287105 - seems like it would be better to take this "simple patch" until the core bug is fixed. Mano, what do you think?
Comment 10•17 years ago
|
||
This applies on top of bug 440702, so maybe this bug should depend on that bug instead of blocking it. This is just a hack until the core issue is fixed (but at least it's better then adding new hacks everytime this problem occurs).
Attachment #334809 -
Flags: review?(mano)
Reporter | ||
Updated•17 years ago
|
Attachment #334809 -
Flags: review?(mano) → review+
Reporter | ||
Comment 11•17 years ago
|
||
Comment on attachment 334809 [details] [diff] [review]
patch - same as "simple patch" but this also removes the hack added in 440702 (checked in)
I'm fine with taking this as a better wallpaper, r=mano. Please don't close the bug though.
Updated•17 years ago
|
Keywords: checkin-needed
Comment 12•17 years ago
|
||
Comment on attachment 334809 [details] [diff] [review]
patch - same as "simple patch" but this also removes the hack added in 440702 (checked in)
http://hg.mozilla.org/mozilla-central/rev/fea1965d16bc
Attachment #334809 -
Attachment description: patch - same as "simple patch" but this also removes the hack added in 440702 → patch - same as "simple patch" but this also removes the hack added in 440702 (checked in)
Updated•17 years ago
|
Assignee: twanno → nobody
Component: Toolbars and Toolbar Customization → XUL
Keywords: checkin-needed
Product: Toolkit → Core
QA Contact: toolbars → xptoolkit.widgets
Updated•17 years ago
|
Status: ASSIGNED → NEW
Comment 13•17 years ago
|
||
Seems like we should add a comment referencing this bug to the code added in customizeToolbar.js, no?
Comment 14•17 years ago
|
||
(In reply to comment #13)
> Seems like we should add a comment referencing this bug to the code added in
> customizeToolbar.js, no?
done: http://hg.mozilla.org/mozilla-central/rev/e28f00d256d1
Comment 15•4 years ago
|
||
Hey Mano,
Can you still reproduce this or should we close it? The test case isn't working for me.
Flags: needinfo?(asaf)
Comment 16•3 years ago
|
||
Redirect a needinfo that is pending on an inactive user to the triage owner.
:enndeakin, since the bug has high severity, could you have a look please?
For more information, please visit auto_nag documentation.
Flags: needinfo?(asaf) → needinfo?(enndeakin)
Comment 17•3 years ago
|
||
Going to redirect to dao on the off-chance he remembers about this.
Flags: needinfo?(enndeakin) → needinfo?(dao+bmo)
Comment 18•3 years ago
|
||
In the process of migrating remaining bugs to the new severity system, the severity for this bug cannot be automatically determined. Please retriage this bug using the new severity system.
Severity: major → --
You need to log in
before you can comment on or make changes to this bug.
Description
•