Closed
Bug 180512
Opened 22 years ago
Closed 21 years ago
Textboxes do not support context attribute overriding default context menu
Categories
(Core :: XUL, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: WeirdAl, Assigned: hyatt)
References
()
Details
Attachments
(3 files, 3 obsolete files)
(deleted),
application/vnd.mozilla.xul+xml
|
Details | |
(deleted),
patch
|
timeless
:
review+
jag+mozilla
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
timeless
:
review+
jag+mozilla
:
superreview+
|
Details | Diff | Splinter Review |
The XUL textbox widget's bindings do not inherit the context menu from the
textbox element. This makes life a little difficult for situations when we wish
to override the default menu.
Workaround involves textbox.inputField.parentNode.setAttribute("context",
textbox.getAttribute("context"));
Patch is really simple, but I left it at home. Basically:
- <xul:hbox class="textbox-input-box" flex="1">
+ <xul:hbox class="textbox-input-box" flex="1" xbl:inherits="context">
...
- <xul:hbox class="textbox-input-box" flex="1">
+ <xul:hbox class="textbox-input-box" flex="1" xbl:inherits="context">
...
- <content context="_child">
+ <content context="_child" inherits="context">
Comment 1•22 years ago
|
||
Reporter | ||
Comment 2•22 years ago
|
||
That'll do it... thanks to sp3k for making me look at LXR for more recent
changes, so we don't leave out the timed-textbox fix.
Reporter | ||
Comment 3•22 years ago
|
||
Comment on attachment 106530 [details] [diff] [review]
diff -u version of comments.
requesting review. This patch hasn't bitrotted yet.
Attachment #106530 -
Flags: review?(timeless)
Reporter | ||
Comment 4•22 years ago
|
||
Comment on attachment 106530 [details] [diff] [review]
diff -u version of comments.
>@@ -100,11 +100,11 @@
> </handlers>
> </binding>
>
> <binding id="timed-textbox" extends="chrome://global/content/bindings/textbox.xml#textbox">
> <content>
>- <xul:hbox class="textbox-input-box" flex="1">
>+ <xul:hbox class="textbox-input-box" flex="1" xbl:inherits="context">
> <html:input class="textbox-input" flex="1" anonid="input"
> xbl:inherits="onfocus,onblur,value,type,maxlength,disabled,size,readonly,tabindex"/>
> </xul:hbox>
> </content>
> <implementation>
Oops, we need to remove this part. Bug 197486 removed the content element for
the binding.
Reporter | ||
Comment 5•22 years ago
|
||
Attachment #106530 -
Flags: superreview?(jaggernaut)
Attachment #106530 -
Flags: review?(timeless)
Attachment #106530 -
Flags: review+
Reporter | ||
Updated•21 years ago
|
Attachment #106530 -
Flags: superreview?(jaggernaut) → superreview?(bzbarsky)
Comment 6•21 years ago
|
||
Comment on attachment 106530 [details] [diff] [review]
diff -u version of comments.
This needs r or sr from a module owner or peer for this code...
Attachment #106530 -
Flags: superreview?(bzbarsky) → superreview?(jaggernaut)
Comment 7•21 years ago
|
||
New patch synch'ed up with trunk, please. And make sure it works for all three
textbox types.
Reporter | ||
Comment 8•21 years ago
|
||
This testcase covers:
<textbox/>
<textbox multiline="true"/>
<textbox type="timed"/>
<textbox type="autocomplete"/>
chrome://global/content/xul.css indicates there are no other textbox bindings.
Attachment #125468 -
Attachment is obsolete: true
Reporter | ||
Comment 9•21 years ago
|
||
jag: good call making me check; in the process I forgot about autocomplete.
Attachment #106530 -
Attachment is obsolete: true
Reporter | ||
Comment 10•21 years ago
|
||
Comment on attachment 125876 [details] [diff] [review]
patch, including autocomplete
Updated patch to reflect:
(1) removed patch code for timed textbox, per comment 4
(2) included patch code for chrome://global/content/autocomplete.xml
Attachment #125876 -
Flags: superreview?(jaggernaut)
Attachment #125876 -
Flags: review?(timeless)
Reporter | ||
Updated•21 years ago
|
Attachment #106530 -
Flags: superreview?(jaggernaut)
Attachment #106530 -
Flags: review+
Attachment #125876 -
Flags: review?(timeless) → review+
Comment 11•21 years ago
|
||
Comment on attachment 125876 [details] [diff] [review]
patch, including autocomplete
<binding id="input-box">
- <content context="_child">
+ <content context="_child" inherits="context">
Shouldn't that be xbl:inherits? Right now, that attribute is getting ignored,
which is I think exactly what we want here, since the context="_child" won't
(or shouldn't) overwrite the context specified (or in this case inherited onto)
the content it's getting bound to.
Attachment #125876 -
Flags: superreview?(jaggernaut) → superreview-
Reporter | ||
Comment 12•21 years ago
|
||
http://www.mozilla.org/projects/xbl/xbl.html#anonymous-attributes
I disagree with that; the element the inherits attribute applies to in this case
is already in the XBL namespace. The spec itself does not explicitly require
the prefix.
The attribute is not getting ignored. Interestingly enough, if you look at it
in DOM Inspector, apparently the xbl prefix applies anyway...
More significantly, though, I tried running my patch without the input-box
binding being altered, and to my surprise, the context menu was still replaced!
More to the point, adding in the context attribute to the other bindings renders
the need for the class attribute of those bindings' <xul:hbox/> elements
obsolete. After a little research, I discovered <xul:hbox
class="textbox-input-box"/> is there specifically to create the context menu,
and that's all.
Reporter | ||
Comment 13•21 years ago
|
||
same as attachment 125876 [details] [diff] [review], minus the code which jag denied sr= on.
Attachment #125876 -
Attachment is obsolete: true
Reporter | ||
Updated•21 years ago
|
Attachment #125978 -
Flags: superreview?(jaggernaut)
Attachment #125978 -
Flags: review?(timeless)
Attachment #125978 -
Flags: review?(timeless) → review+
Comment 14•21 years ago
|
||
> http://www.mozilla.org/projects/xbl/xbl.html#anonymous-attributes
>
> I disagree with that; the element the inherits attribute applies to in this
> case is already in the XBL namespace. The spec itself does not explicitly
> require the prefix.
The attribute isn't in the xbl namespace:
http://www.w3.org/TR/xml-names11/#defaulting
"A default namespace declaration applies to to all unprefixed element names
within its scope. Default namespace declarations do not apply directly to
attribute names; the interpretation of unprefixed attributes is determined by
the element on which they appear."
For XBL we've chosen to require the explicit XBL namespace prefixing for
|inherits| (the xbl spec needs to be corrected to make that clear).
http://lxr.mozilla.org/seamonkey/source/content/xbl/src/nsXBLPrototypeBinding.cpp#988
> The attribute is not getting ignored. Interestingly enough, if you look at
> it in DOM Inspector, apparently the xbl prefix applies anyway...
I'll take a look. DOM Inspector might be getting this wrong.
> More significantly, though, I tried running my patch without the input-box
> binding being altered, and to my surprise, the context menu was still
> replaced!
I'm not surprised, see comment 11. Or let me try to explain it differently:
what's happening is that the |context="_child"| on the "input-box" binding only
gets copied into the element it's bound to if it doesn't have that attribute
already. Because you've added |xbl:inherits="context"| to the <hbox>, the
attribute, if specified on the <textbox>, will exist and the default value for
it ("_child") won't get copied over from the "input-box" binding.
> More to the point, adding in the context attribute to the other bindings
> renders the need for the class attribute of those bindings' <xul:hbox/>
> elements obsolete. After a little research, I discovered <xul:hbox
> class="textbox-input-box"/> is there specifically to create the context menu,
> and that's all.
You still need the "input-box" binding bound to those <hbox>en. If you remove
the class="textbox-input-box", you won't have a default context menu anymore
(neither the "_child" default nor the actual menu items).
Comment 15•21 years ago
|
||
Comment on attachment 125978 [details] [diff] [review]
patch, without altering <binding id="textbox-input-box"/>
sr=jag
Attachment #125978 -
Flags: superreview?(jaggernaut) → superreview+
Comment 16•21 years ago
|
||
Checked in.
Comment 17•21 years ago
|
||
So, do we need the same thing for editable menulists?
Reporter | ||
Comment 18•21 years ago
|
||
Neil: good of you to point that out. Technically, that would merit a separate
bug, but it'd be a waste of everyone's time and Bugzilla's resources. The
patch is trivial anyway.
Reporter | ||
Updated•21 years ago
|
Attachment #126065 -
Flags: superreview?(jaggernaut)
Attachment #126065 -
Flags: review?(timeless)
Attachment #126065 -
Flags: review?(timeless) → review+
Comment 19•21 years ago
|
||
Comment on attachment 126065 [details] [diff] [review]
patch for <menulist editable="true"/>
sr=jag
Attachment #126065 -
Flags: superreview?(jaggernaut) → superreview+
Comment 20•21 years ago
|
||
Fix checked in.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: shrir → xptoolkit.widgets
Reporter | ||
Comment 21•15 years ago
|
||
FYI, bug 312869 is a followup to this (because this bug was actually a bad idea).
You need to log in
before you can comment on or make changes to this bug.
Description
•