Hide <form>'s named property getter on isolated context
Reported by
gongd...@gmail.com,
Oct 20
|
|||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.67 Safari/537.36 Steps to reproduce the problem: 1. Say a <form> has a child: <input name="parentElement" /> and a user clicks the <input> 2. now a web extension needs to find some special types of ancestors of the click target, 3. then it will be trapped into a dead loop if it use `element = element.parentElement` What is the expected behavior? For a <form>, <frameset>, <embed> or <object>, its parentElement/parentNode/previousSibling/childNodes/innerText members and contains/getAttribute/focus/blur member functions work JUST LIKE other common elements' (e.g. <div> and <span>), **at least on web extensions' isolated contexts**. What went wrong? If a <form>/<frameset> has some special named children, visiting its properties may cause an JS exception or a dead loop. This is too annoying because, for web extensions, any builtin property name might get overriden, which means none of properties of an arbitrary element is reliable !!! Did this work before? No Chrome version: 70.0.3538.67 Channel: stable OS Version: 10.0 Flash Version:
,
Oct 21
,
Oct 22
A small fix: sicne Chrome 70, the named property getter on <frameset> has been removed, so on common webpages only `<form>` can cause such a problem. I'm glad if all named_property_getters on <form>, <frameset> and even `Window` would be hidden, but if it's not easy to make a decision, I would like to get <form>'s getter hidden as soon as possible. Here's an example to see how hard it is to work around this annoying feature: https://github.com/gdh1995/vimium-c/blob/defa59a4a3b7af9b73682c6f88449ce281969e99/lib/dom_utils.ts#L59 . The function "GetParent_" is to access the real element.parentElement, and I have to check if a <form> has overridden `parentNode` or `parentElement`: * if none (parentNode == parentElement) is overridden, then it's the simplest case * if only one of them is overridden, then I use `Element::contains` to tell the "correct" one * if both are not the real parent, then I have to use `Object.getOwnPropertyDescriptor` to get the getter of `Node::parentNode`, and call the getter on a <form> element. Another small example is: at other places where I don't need an accurate result, I have to test `element::*` before using them, like `typeof element.getClientRects === "function" ? element.getClientRects() : []` (https://github.com/gdh1995/vimium-c/blob/defa59a4a3b7af9b73682c6f88449ce281969e99/lib/dom_utils.ts#L128). In the extension above, I'm reworking code to try to acquire such a safety, but up to now only the dom_utils.ts is almost finished, but in its "content" folder, there're 9 files to be review and fix !!!
,
Oct 25
Thanks for filing the issue! Tried testing the issue on chrome reported version# 70.0.3538.67 using Windows-10 with steps mentioned below: 1) Launched chrome reported version and navigated to URL: https://jsfiddle.net/bpf2syck/, provided in comment# 1 2) On navigating, able to see a popup with "An embedded page at fiddle.jshell.net says", and clicked on "OK" 3) Able to a text field on the page @Reporter: Please find the above information and provide your feedback on it, if possible provide the screencast of the issue which help in better understanding and further triaging it in better way. Thanks!
,
Oct 25
I opened the page on JSFiddle for several times, and the results are always like the attached 1.png. Today I updated my Chrome to 70.0.3538.77 stable and the test pages works all the same. Some further tests on Console is like the attached 2.png. I mean, I just want this "[OverrideBuiltins] getter (DOMString name)" on <form> (and <embed> / <object>) to exist on ONLY the main v8 world, so that extension scripts are safe enough to access `parentElement` on all nodes.
,
Oct 25
Sorry the last commit has no attachments. This `1.png` is what I got on the test JSFiddle page. The `2.png` is some results on Console of the Developer Tools.
,
Oct 25
Thank you for providing more feedback. Adding the requester to the cc list. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 25
I've created https://jsfiddle.net/bpf2syck/3/ to show what a mass overridden properties can cause - enter dead loops when itering the DOM tree + throw unexpected "*** is not a function" exceptions. On the JSFiddle page, the result iframe has an area to show normal accessing methods and their broken (unexpected) results.
,
Oct 26
,
Oct 26
Binding team, is it possible to turn off [OverrideBuiltins] only for isolated context?
,
Oct 26
This seems like something that should be decided for the web platform as a whole, not just for extensions contexts. Generally, we don't want to fragment the behavior of extensions contexts from the web.
,
Oct 27
Of course, The solution to remove "[OverrideBuiltins]" from <form>'s IDL is aggressive in some degree, so here's one of another idea: Chrome may disable the "named property getter" of <form> in most DOM wrapper worlds except the "MainWorld", just like what has been applied on #document, <embed> and <object>. For isolated DOM worlds, I think they can use `<form>.elements[index]` to visit form's controls, so this idea might have less influence. A use-counter is suitable to count how many times other worlds try to access <form>s' form-control properties. From https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/html/html_document.cc?dr=C&g=0&l=103 , we know "HTMLDocument::AddNamedItem" makes only the `document` object in the main world will have named items as its properties, and other worlds has no worries about mistaken visiting actions. From https://cs.chromium.org/chromium/src/third_party/blink/renderer/bindings/core/v8/custom/v8_html_plugin_element_custom.cc?l=46 , we can confirm that named property getters of <embed> and <object> are also working only on the main world. This change on "GetScriptableObjectProperty" was made by commit 1e196e33687bd22a33c8b2116956754b90b29bde, and take effects since Chrome 61.
,
Oct 30
We, bindings team, don't want to do this change.
Generally we don't want to make differences in some specific cases, unless some spec define so.
For this case, especially, you can get <form> from <input> elements, with following prototype chain by yourself.
let input = getElementById('parentElement');
let form = input.__proto__.parentElement;
and we cannot guarantee that the DOM tree is made from author's HTML/JS. Some other extensions may change the DOM tree and they may need named property getters to access elements.
The last case also means that this change can break backward compatibility for such extensions.
It is another reason we don't want to this change.
We know that it is annoying to write prototype chains explicitly, but we believe you don't need so much updates in practice.
So let us close this as WontFix. Feel free to reopen this, if you have any comments or some suggestions.
,
Oct 30
@peria, thanks, but sorry I'm afraid I didn't express myself clearly. What I want is a convenient method in extension contexts to access the real built-in properties of a <form>'s JS wrapper instance, while currently the named property getter is OverrideBuiltin and Cross-DOM-World. I also know this request might break backward compatibility in some degree, but `OverrideBuiltin` is indeed very annoying. I wonder if a special extension permission in manifest or a chrome flag can help. If there's indeed no way to solve this, I wonder whether there have been any issues on https://github.com/whatwg/html , and if not, maybe I'll open one. BTW, both `input.__proto__.parentElement` (your mentioned above) and `form.__proto__.parentElement` are not expected usages. They only throw `TypeError: Illegal invocation` on Chrome 70, or return `undefined` on Chrome 35. In fact, I haven't find a solution on old versions of Chrome to access the real parentElement of a <form>. |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by leose...@rambler.ru
, Oct 21