Closed
Bug 303727
Opened 19 years ago
Closed 19 years ago
Supply a supported way to access the nsIEditor of HTML input/textarea
Categories
(Core :: DOM: Core & HTML, enhancement)
Core
DOM: Core & HTML
Tracking
()
VERIFIED
FIXED
People
(Reporter: bryner, Assigned: bryner)
References
Details
(Keywords: fixed1.8)
Attachments
(3 files, 1 obsolete file)
9.03 KB,
patch
|
jst
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
4.08 KB,
patch
|
jst
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
8.80 KB,
patch
|
asa
:
approval1.8b5+
|
Details | Diff | Splinter Review |
Currently, there's no supported way to access the inner <div> for HTML input/textarea elements. There are some interesting use cases for doing this (see the spellcheck feature of Google's Firefox toolbar), but right now it requires writing native code that depends on unfrozen interfaces. I'm proposing we add a new interface which is supported by input and textarea elements and provides access to the inner div element. Because of the possibility of using this to trick the user, the functionality would only be made available to chrome.
Assignee | ||
Comment 1•19 years ago
|
||
Attachment #191832 -
Flags: superreview?(jst)
Attachment #191832 -
Flags: review?(jst)
Comment 2•19 years ago
|
||
Comment on attachment 191832 [details] [diff] [review] patch +/** + * This interface is implemented by elements which have inner editable content, + * such as HTML input and textarea. + * + * @status FROZEN +*/ + +[scriptable, uuid(c4a71f8e-82ba-49d7-94f9-beb359361072)] +interface nsIDOMNSEditableElement : nsISupports +{ + readonly attribute nsIDOMElement innerDiv; +}; Exposing internals like the fact that we've got a div inside our editable elements in a frozen interface seems like something that could come back and haunt us later on... What's the actual need for this? Who would need this inner div (and what if there are more than one?), wouldn't it make more sense to return the editor for the element instead? And what if there's more than one, like with some of the whatwg widgets I could see us having more than one editor in a single HTML widget... - In content/base/public/nsContentUtils.h + /** + * Get the first native anonymous child element for a node. + */ + static already_AddRefed<nsIDOMElement> + GetAnonymousChildElement(nsIContent *aContent); Given the current use of this, this would IMO fit better into nsGenericHTMLElement, or one of the related classes. I don't see anything in this diff that prevents content code from getting at the inner div if exposed this way, content code can QI just as easily as chrome code can. If you want this to be callable from chrome only, you'd need some nsContentUtils::IsCallerChrome() checks somewhere...
Attachment #191832 -
Flags: superreview?(jst)
Attachment #191832 -
Flags: superreview-
Attachment #191832 -
Flags: review?(jst)
Attachment #191832 -
Flags: review-
Assignee | ||
Comment 3•19 years ago
|
||
(In reply to comment #2) > haunt us later on... What's the actual need for this? Who would need this inner > div (and what if there are more than one?), wouldn't it make more sense to > return the editor for the element instead? And what if there's more than one, > like with some of the whatwg widgets I could see us having more than one editor > in a single HTML widget... I gave a use case in my initial comment... basically it's useful for any kind of annotation that someone could want to do to our text inputs. > return the editor for the element instead? And what if there's more than one, > like with some of the whatwg widgets I could see us having more than one editor > in a single HTML widget... Example? > Given the current use of this, this would IMO fit better into > nsGenericHTMLElement, or one of the related classes. Sure, that's fine. > I don't see anything in this diff that prevents content code from getting at > the inner div if exposed this way, content code can QI just as easily as chrome > code can. If you want this to be callable from chrome only, you'd need some > nsContentUtils::IsCallerChrome() checks somewhere... If I try this, I get: Error: uncaught exception: Permission denied to get property HTMLDivElement.style I'm pretty sure native-anonymous content is already protected from modification by untrusted content.
Assignee | ||
Comment 4•19 years ago
|
||
It's not great, because nsIEditor isn't frozen, but it's better than where we're at now.
Attachment #191832 -
Attachment is obsolete: true
Attachment #194617 -
Flags: superreview?(jst)
Attachment #194617 -
Flags: review?(jst)
Comment 5•19 years ago
|
||
Comment on attachment 194617 [details] [diff] [review] another try Yeah, that's IMO much better, even if we're exposing non-frozen interfaces to scripts that really care. r+sr=jst
Attachment #194617 -
Flags: superreview?(jst)
Attachment #194617 -
Flags: superreview+
Attachment #194617 -
Flags: review?(jst)
Attachment #194617 -
Flags: review+
Assignee | ||
Comment 6•19 years ago
|
||
Checked in on the trunk.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 7•19 years ago
|
||
Comment on attachment 194617 [details] [diff] [review] another try I'd really like to get this in for 1.5, even though it's clearly an enhancement. Here's my thinking: 1. It does not affect existing code paths 2. The new functionality is only available to chrome, so it shouldn't be a security problem. 3. It provides a much-needed piece of functionality for extension authors.
Attachment #194617 -
Flags: approval1.8b5?
Comment 8•19 years ago
|
||
Comment on attachment 194617 [details] [diff] [review] another try It's late in the game and we have a lot more to be concerned with right now. We realize you were looking for an exception to the "it's too late for features" and this got a lot of discussion before I minused it. Hopefully Google can do something with native code.
Attachment #194617 -
Flags: approval1.8b5? → approval1.8b5-
Comment 9•19 years ago
|
||
Since nsIDOMNSHTMLInputElement and nsIDOMNSHTMLTextAreaElement are logically identical is there a case for moving everything into nsIDOMNSEditableElement?
Comment 10•19 years ago
|
||
> It's late in the game and we have a lot more to be concerned with right now.
> We realize you were looking for an exception to the "it's too late for
> features" and this got a lot of discussion before I minused it. Hopefully
> Google can do something with native code.
Asa: I think we should take this patch for Firefox 1.5. It provides a good,
stable API that has very little associated risk. It is not affecting existing
codepaths. It simply exposes a property that is otherwise difficult to access.
This patch specifically would enable a certain toolbar extension to avoid having
to code to a bunch of unfrozen interfaces (e.g., nsIDocShell, nsIPresShell, and
nsIContent). Moreover, those are interfaces that we are very likely to want to
change as we apply other patches to Gecko during the run-up to Firefox 1.5 final
(e.g., see the proposed patches to improve the memory utilization of fastback).
I think it would be silly for us to not accept this patch on the grounds that we
are done taking features. This is not just a feature, it's an API that makes it
easier for extensions to do cool stuff with less risk of being broken by core
changes to Gecko. We like APIs that are easy to support in the future! ;-)
Flags: blocking1.8b5?
Comment 11•19 years ago
|
||
Comment on attachment 194617 [details] [diff] [review] another try Please see my previous comment in this bug. I think we should include this patch for Firefox 1.5b2.
Attachment #194617 -
Flags: approval1.8b5- → approval1.8b5?
Updated•19 years ago
|
Summary: Supply a supported way to access anonymous <div> of HTML input/textarea → Supply a supported way to access the nsIEditor of HTML input/textarea
Comment 12•19 years ago
|
||
So Brendan said that he was in favor of this patch on the 1.8 branch provided that we hide the ".editor" property from content. From the patch it appears that we're just making the property throw if called by content. So, how easy would it be to hide it altogether?
Comment 13•19 years ago
|
||
I'm not sure there's an obvious way to hide it, given XPConnect's interface flattening. Once someone QIs to this interface on an actual content node, the property will be on the wrapper... That said, should <xul:textbox> implement this interface, perhaps? And should code that has UniversalXPConnect but isn't chrome be able to access this?
Comment 14•19 years ago
|
||
Ah, brendan was suggesting hiding via classinfo... That could maybe be done. You'd have to only resolve and get the prop if the caller has the right privileges, and make sure that it's using JSPROP_SHARED so it doesn't get a slot on the object. And of course that way it'll only be accessible from JS, not from C++ or python or whatever..
Comment 15•19 years ago
|
||
(In reply to comment #14) > Ah, brendan was suggesting hiding via classinfo... > > That could maybe be done. You'd have to only resolve and get the prop if the > caller has the right privileges, and make sure that it's using JSPROP_SHARED so > it doesn't get a slot on the object. And of course that way it'll only be > accessible from JS, not from C++ or python or whatever.. The way to hide this would IMO be to make the new interface a standalone interface that inherits nothing and nothing inherits from. That means one more vtable where we implement this, but it also means that this property is hidden until someone QI's to this new interface, which would only be done by extensions that use this feature... Sure, this could still break some random pages out there, *if* an extension is installed that QI's all textareas and inputs on page load... That would IMO be acceptable for the 1.8 branch.
Comment 16•19 years ago
|
||
If you implement as comment 15 suggests - i.e. hidding the element from content we'll approve for 1.8b5.
Updated•19 years ago
|
Flags: blocking1.8b5? → blocking1.8b5+
Updated•19 years ago
|
Attachment #194617 -
Flags: approval1.8b5?
Assignee | ||
Comment 17•19 years ago
|
||
Attachment #196883 -
Flags: superreview?(jst)
Attachment #196883 -
Flags: review?(jst)
Comment 18•19 years ago
|
||
Comment on attachment 196883 [details] [diff] [review] trunk patch to require explicit QI to nsIDOMNSEditableElement >-[scriptable, uuid(8aa7dadc-73f1-416e-9d1e-15bc63226938)] >-interface nsIDOMNSHTMLTextAreaElement : nsIDOMNSEditableElement >+[scriptable, uuid(ca066b44-9ddf-11d3-bccc-0060b0fc76bd)] >+interface nsIDOMNSHTMLTextAreaElement : nsISupports Shouldn't this be a backout rather than a new uuid?
Assignee | ||
Comment 19•19 years ago
|
||
It is. This patch restores the original uuids. (In reply to comment #18) > (From update of attachment 196883 [details] [diff] [review] [edit]) > >-[scriptable, uuid(8aa7dadc-73f1-416e-9d1e-15bc63226938)] > >-interface nsIDOMNSHTMLTextAreaElement : nsIDOMNSEditableElement > >+[scriptable, uuid(ca066b44-9ddf-11d3-bccc-0060b0fc76bd)] > >+interface nsIDOMNSHTMLTextAreaElement : nsISupports > Shouldn't this be a backout rather than a new uuid? >
Comment 20•19 years ago
|
||
Comment on attachment 196883 [details] [diff] [review] trunk patch to require explicit QI to nsIDOMNSEditableElement r+sr=jst, please put a branch diff in the bug before checking this in on the branch (since that diff will be the sum of both these diffs).
Attachment #196883 -
Flags: superreview?(jst)
Attachment #196883 -
Flags: superreview+
Attachment #196883 -
Flags: review?(jst)
Attachment #196883 -
Flags: review+
Assignee | ||
Comment 21•19 years ago
|
||
Attachment #196964 -
Flags: approval1.8b5?
Comment 22•19 years ago
|
||
(In reply to comment #19) >It is. This patch restores the original uuids. Sorry, it would have helped if I had looked at the right .idl :-[
Updated•19 years ago
|
Attachment #196964 -
Flags: approval1.8b5? → approval1.8b5+
Comment 23•19 years ago
|
||
This is pretty cool! editor = (xulTextbox.inputField instanceof Components.interfaces.nsIDOMNSEditableElement) ? xulTextbox.inputField.editor : null
Status: RESOLVED → VERIFIED
Comment 24•19 years ago
|
||
Going to add that to the XBL?
Comment 25•19 years ago
|
||
Yes. Bug 312867.
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•