[PATCH 0/1] MR8586: mshtml: Fix node's onclick not responding.
When installing the photoshop-setup.exe version of Photoshop 2025, there is no response after clicking the continue button. The reasons are as follows: As a property of the node, setting events such as onclick on the node does not respond when clicked. The following jscript code was executed during the installation of Photoshop 2025: $("#continueButton").attr("onclick", "continueBtnClicked('installOptionsContinueClicked'); return false;"); -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8586
From: yaoyongjie <yaoyongjie(a)uniontech.com> When installing the photoshop-setup.exe version of Photoshop 2025, there is no response after clicking the continue button. The reasons are as follows: As a property of the node, setting events such as onclick on the node does not respond when clicked. The following jscript code was executed during the installation of Photoshop 2025: $("#continueButton").attr("onclick", "continueBtnClicked('installOptionsContinueClicked'); return false;"); --- dlls/mshtml/mutation.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/dlls/mshtml/mutation.c b/dlls/mshtml/mutation.c index 92fc97a03d9..7429fb24678 100644 --- a/dlls/mshtml/mutation.c +++ b/dlls/mshtml/mutation.c @@ -784,9 +784,31 @@ static void NSAPI nsDocumentObserver_AttributeWillChange(nsIDocumentObserver *if { } +/** + * namespace mozilla { + * namespace dom { + * class Element : public FragmentOrElement + * class FragmentOrElement : public nsIContent + * } + *} +*/ static void NSAPI nsDocumentObserver_AttributeChanged(nsIDocumentObserver *iface, nsIDocument *aDocument, - void *aElement, LONG aNameSpaceID, nsIAtom *aAttribute, LONG aModType, const nsAttrValue *aOldValue) + /*mozilla::dom::Element*/ void *aElement, LONG aNameSpaceID, nsIAtom *aAttribute, LONG aModType, const nsAttrValue *aOldValue) { + HTMLDocumentNode *This = impl_from_nsIDocumentObserver(iface); + nsIDOMElement *nselem = NULL; + nsIContent *nselem_node; + nsresult nsres; + + TRACE("%p, %p, %p\n", iface, This->dom_document, aElement); + + if (aElement) + { + nselem_node = (nsIContent *)aElement; + nsres = nsIContent_QueryInterface(nselem_node, &IID_nsIDOMElement, (void **)&nselem); + if(NS_SUCCEEDED(nsres) && nselem) + check_event_attr(This, nselem); + } } static void NSAPI nsDocumentObserver_NativeAnonymousChildListChange(nsIDocumentObserver *iface, nsIDocument *aDocument, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8586
Jacek Caban (@jacek) commented about dlls/mshtml/mutation.c:
+*/ static void NSAPI nsDocumentObserver_AttributeChanged(nsIDocumentObserver *iface, nsIDocument *aDocument, - void *aElement, LONG aNameSpaceID, nsIAtom *aAttribute, LONG aModType, const nsAttrValue *aOldValue) + /*mozilla::dom::Element*/ void *aElement, LONG aNameSpaceID, nsIAtom *aAttribute, LONG aModType, const nsAttrValue *aOldValue) { + HTMLDocumentNode *This = impl_from_nsIDocumentObserver(iface); + nsIDOMElement *nselem = NULL; + nsIContent *nselem_node; + nsresult nsres; + + TRACE("%p, %p, %p\n", iface, This->dom_document, aElement); + + if (aElement) + { + nselem_node = (nsIContent *)aElement; + nsres = nsIContent_QueryInterface(nselem_node, &IID_nsIDOMElement, (void **)&nselem); You may just treat it as `nsISupports`, like:
nsres = nsISupports_QueryInterface(aElement, &IID_nsIDOMElement, (void **)&nselem);
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8586#note_110173
Jacek Caban (@jacek) commented about dlls/mshtml/mutation.c:
- void *aElement, LONG aNameSpaceID, nsIAtom *aAttribute, LONG aModType, const nsAttrValue *aOldValue) + /*mozilla::dom::Element*/ void *aElement, LONG aNameSpaceID, nsIAtom *aAttribute, LONG aModType, const nsAttrValue *aOldValue) { + HTMLDocumentNode *This = impl_from_nsIDocumentObserver(iface); + nsIDOMElement *nselem = NULL; + nsIContent *nselem_node; + nsresult nsres; + + TRACE("%p, %p, %p\n", iface, This->dom_document, aElement); + + if (aElement) + { + nselem_node = (nsIContent *)aElement; + nsres = nsIContent_QueryInterface(nselem_node, &IID_nsIDOMElement, (void **)&nselem); + if(NS_SUCCEEDED(nsres) && nselem) + check_event_attr(This, nselem); `check_event_attr` is not exactly what we need here. It iterates over all attributes, so it would recreate and replace existing non-related event handlers. We'd probably want to factor out handling of a single attribute from it instead. You may get the name of the changed attribute from `nsIAtom` passed to `AttributeChanged` with `ScriptableToString` and the [attached](/uploads/4927195f5b520b7aceef83ab39a89d24/nsiatom.diff) `nsIAtom` declaration.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8586#note_110174
The following jscript code was executed during the installation of Photoshop 2025: ``` $("#continueButton").attr("onclick", "continueBtnClicked('installOptionsContinueClicked'); return false;"); ```
I'm guessing that ends up calling `Element.setAttribute()`. For legacy (pre-IE9) modes we support that by intercepting `setAttribute` and friends themself. Theoretically we could do that for IE9+ too, but I like your solution more, as it's more flexible. CC @insn. Please add a test case to `events.js`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8586#note_110175
On Thu Jul 17 15:33:57 2025 +0000, Jacek Caban wrote:
The following jscript code was executed during the installation of Photoshop 2025: ``` $("#continueButton").attr("onclick", "continueBtnClicked('installOptionsContinueClicked'); return false;"); ``` I'm guessing that ends up calling `Element.setAttribute()`. For legacy (pre-IE9) modes we support that by intercepting `setAttribute` and friends themself. Theoretically we could do that for IE9+ too, but I like your solution more, as it's more flexible. CC @insn. Please add a test case to `events.js`. I'm not sure what happens with elements not part of the DOM tree, they might not call `AttributeChanged`. If that's true and we go this route, we might need to intercept them being added to the document to parse their attributes there.
Can you please test/add tests for detached elements (not inserted into DOM tree) as well and see what happens? @yaoyongjie -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8586#note_110195
we might need to intercept them being added to the document to parse their attributes there.
We do that in `BindToDocument()`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8586#note_110206
On Thu Jul 17 19:52:21 2025 +0000, Jacek Caban wrote:
we might need to intercept them being added to the document to parse their attributes there. We do that in `BindToDocument()`. Ah right, for some reason I thought it was only used during parsing.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8586#note_110207
On Thu Jul 17 11:13:58 2025 +0000, Jacek Caban wrote:
`check_event_attr` is not exactly what we need here. It iterates over all attributes, so it would recreate and replace existing non-related event handlers. We'd probably want to factor out handling of a single attribute from it instead. You may get the name of the changed attribute from `nsIAtom` passed to `AttributeChanged` with `ScriptableToString` and the [attached](/uploads/4927195f5b520b7aceef83ab39a89d24/nsiatom.diff) `nsIAtom` declaration. I understand what you mean, I should only handle the onclick attribute, but I cannot obtain the attribute name of nsIAtom. There is no export of the ScriptableToString function in xul.dll of wine-gecko 2.47.4.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8586#note_110464
On Thu Jul 17 19:58:41 2025 +0000, Gabriel Ivăncescu wrote:
Ah right, for some reason I thought it was only used during parsing. @jacek Ok, I will add test case to events.js. You are right, ends up calling `Element.setAttribute()`, but don't be as the value of "onclick" as event, it's just been as simple string value. The following is the output log：
0024:trace:mshtml:HTMLElement_setAttribute (09555C70)->(L"onclick" 0012E4A0 {VT_BSTR: L"continueBtnClicked('installOptionsContinueClicked'); return false;"} 00000000) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8586#note_110465
On Mon Jul 21 02:18:10 2025 +0000, Yongjie Yao wrote:
I understand what you mean, I should only handle the onclick attribute, but I cannot obtain the attribute name of nsIAtom. There is no export of the ScriptableToString function in xul.dll of wine-gecko 2.47.4. It's a COM method, so there is no export. With the patch I attached to the previous comment, you should be able to do `nsIAtom_ScriptableToString(aAttribute, &nsstr)`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8586#note_110593
Superseded by !9976. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8586#note_128071
This merge request was closed by Jacek Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8586
participants (5)
-
Gabriel Ivăncescu -
Jacek Caban (＠jacek) -
Jacek Caban (＠jacek)
-
yaoyongjie
-
Yongjie Yao (＠yaoyongjie)