1789435 - With middlemouse.paste enabled: Middle-click on input field triggers TypeError: can't access property "startsWith", node.closest(...).constructor.name is undefined

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Rob Wu [:robwu]]

STR:

1. Visit any page that features an input field, e.g. the default about:home or duckduckgo.com.
2. Use the middle mouse wheel to click on it.
3. Open the global browser console or the browser toolbox to view error messages from the content process.

Expected:

• no errors

Actual:

• TypeError: can't access property "startsWith", node.closest(...).constructor.name is undefined AutoScrollChild.jsm:64:12

The logic in AutoScrollChild.jsm was introduced by bug 1716883.

Usually, node.constructor.name would be the name of the constructor. But in this case, some weirdness is going on with XrayWrappers. The constructor does not point to the actual HTMLDivElement class, but some wrapped version that does not have the name property. This is not visible through the tab's devtools: i.e. if you inspect the element with the devtools and then type $0.constructor.name, you'd get the expected string.

To inspect the actual value, you'd have to open the debugger of the Browser Toolbox, put a breakpoint at AutoScrollChild.jsm:64 and then you can confirm that node.constructor.name is undefined. And when the XrayWrapper is unwrapped (e.g. with .wrappedJSObject), the expected string is seen (i.e. node.wrappedJSObject.constructor.name and node.constructor.wrappedJSObject.name is HTMLDivElement).

To avoid the issue in this case, the HTMLTextAreaElement.isInstance and HTMLInputElement.isInstance methods could be used instead of checking the constructor name. Or if you'd like to get a string for some reason, use Cu.getClassName(node, true) instead of .constructor.name

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1716883

:Gijs, since you are the author of the regressor, bug 1716883, could you take a look? Also, could you set the severity field?

For more information, please visit auto_nag documentation.

Comment 2

[:Gijs (he/him)]

What is the actual consequence of this error, if any?

Comment 3

[Rob Wu [:robwu]]

(In reply to :Gijs (he/him) from comment #2)

What is the actual consequence of this error, if any?

Just logspam. I reported this surprising error so that there is a explanation somewhere that explains that constructor.name is unreliable when XrayWrappers are involved. The actual code fix to get rid of the error is trivial.

The specific code is supposed to only disable autoscroll logic for input fields in HTML documents, i.e. continue the autoscroll blocking logic in non-HTML documents (SVG?, XUL?).
In practice, since the code has been throwing for all documents (and effectively not triggering the auto scroll logic), the very specific condition may not be required, and it could be reasonable to resolve the bug by removing the ?.constructor.name.startsWith("HTML") altogether.

The code issue is trivial to fix though; at the bottom of the report I wrote:

To avoid the issue in this case, the HTMLTextAreaElement.isInstance and HTMLInputElement.isInstance methods could be used instead of checking the constructor name. Or if you'd like to get a string for some reason, use Cu.getClassName(node, true) instead of .constructor.name

Comment 4

[:Gijs (he/him)]

(In reply to Rob Wu [:robwu] from comment #3)

The code issue is trivial to fix though

Sure, but I can't prioritize it right now.

Comment 5

[Tom S. (please needinfo tschuster)]

I am kinda curious why the XrayWrapper is wrong. This might be fixable. I will try to look at this at some point.

Comment 6

[Tom S. (please needinfo tschuster)]

Attachment: Bug 1789435 - Xray resolve the name of DOM constructors. r?peterv

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The product::component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1716883

Comment 9

[:Gijs (he/him)]

https://searchfox.org/mozilla-central/search?q=constructor.name&path=actor&case=false&regexp=false suggests the autoscroll child is the only child-focused code that does this, so perhaps the simplest thing is to "just" fix the actor and move on.

Comment 10

[:Gijs (he/him)]

Attachment: Bug 1789435 - fix exception in autoscroll code when middle mouse paste is enabled, r?robwu

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ecd6cde6779d
fix exception in autoscroll code when middle mouse paste is enabled, r=robwu

Comment 12

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/ecd6cde6779d