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">

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.

Comment on attachment 106530 [details] [diff] [review] diff -u version of comments. requesting review. This patch hasn't bitrotted yet.

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.

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...

New patch synch'ed up with trunk, please. And make sure it works for all three textbox types.

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.

jag: good call making me check; in the process I forgot about autocomplete.

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

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.

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.

same as attachment 125876 [details] [diff] [review], minus the code which jag denied sr= on.

> 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 on attachment 125978 [details] [diff] [review] patch, without altering <binding id="textbox-input-box"/> sr=jag

Checked in.

So, do we need the same thing for editable menulists?

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.

Comment on attachment 126065 [details] [diff] [review] patch for <menulist editable="true"/> sr=jag

Fix checked in.

FYI, bug 312869 is a followup to this (because this bug was actually a bad idea).