240834 2022-05-23 15:38:37 -0700 Confusing inconsistencies when programmatically applying edit commands to `display:none` elements 2022-05-24 10:30:16 -0700 1 1 1 Unclassified WebKit HTML Editing WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 thorton thorton ap koivisto ntim rniwa webkit-bug-importer wenson_hsieh oldest_to_newest 1871328 0 459689 thorton 2022-05-23 15:38:37 -0700 Created attachment 459689 repro case See attached test case. WebKit has an odd behavior where if you try to `insertText` into a contenteditable display:none element, it rejects the editing command, but if you try to do the same into a *child* of the contenteditable display:none element, it works. *Also* interestingly, while the other browser engines behave internally consistently, they do not agree about the correct behavior: Blink rejects the editing command in both cases. Gecko allows the editing command in both cases. 1871329 1 thorton 2022-05-23 15:40:31 -0700 I believe the root of the inconsistency in WebKit is the `display: none` check in computeEditabilityFromComputedStyle, which only checks the style of the immediate starting node, not whether or not you're in a `display: none` subtree. However, I'm not sure which of the more-consistent behaviors to align us with :) 1871351 2 thorton 2022-05-23 16:53:02 -0700 I *think* that Antti introduced this in https://trac.webkit.org/changeset/160966/webkit 1871356 3 thorton 2022-05-23 17:14:16 -0700 https://bugs.webkit.org/show_bug.cgi?id=133371 suggests that display:none things should be editable. Humorously, while https://trac.webkit.org/changeset/160966/webkit progressed bug 133371 in the case where you're querying isContentEditable on a child of the display:hidden element, it still says NO if you query *exactly* the display:hidden element. Also, both Chrome and Firefox say isContentEditable=true for all elements (the direct display:none element and the children). I believe that the fact that Chrome doesn't allow editing in my original test case does not have to do with editability, but rather with selection behavior in display:none, so not necessarily related to this bug. All of this together, plus the fact that changing this only affects a single test, makes me think that aligning on the Firefox behavior is the way to go. I'm going to prepare a patch to do so. Please shout if you disagree! 1871360 4 rniwa 2022-05-23 17:25:43 -0700 display:none shouldn't affect the result of isContentEditable but I don't think the rest of editing code can deal with the display:none content. If there aren't any editing tests that rely on this, then either approach sounds okay for now. 1871407 5 thorton 2022-05-23 21:02:42 -0700 Pull request: https://github.com/WebKit/WebKit/pull/964 1871528 6 ews-feeder 2022-05-24 10:29:42 -0700 Committed r294753 (250921@main): <https://commits.webkit.org/250921@main> Reviewed commits have been landed. Closing PR #964 and removing active labels. 1871529 7 webkit-bug-importer 2022-05-24 10:30:16 -0700 <rdar://problem/93843189> 459689 2022-05-23 15:38:37 -0700 2022-05-23 15:38:37 -0700 repro case span.html text/html 687 thorton PCFET0NUWVBFIEhUTUw+CjxodG1sPgo8c3R5bGU+CnAgeyBkaXNwbGF5OiBpbmxpbmUtYmxvY2s7 IH0KPC9zdHlsZT4KPGJvZHk+CiAgICBFZGl0aW5nIHdhcyA8cCBjb250ZW50ZWRpdGFibGUgaWQ9 InRhcmdldEEiPnJlamVjdGVkPC9wPiAoZGlyZWN0IGRpc3BsYXk6bm9uZSk8YnIvPgogICAgPHAg Y29udGVudGVkaXRhYmxlIGlkPSJ0YXJnZXRCIj5FZGl0aW5nIHdhcyA8c3BhbiBpZD0icmVwbGFj ZW1lbnRCIj5yZWplY3RlZDwvc3Bhbj4uPC9wPiAocGFyZW50IGRpc3BsYXk6bm9uZSkKPHNjcmlw dD4KZnVuY3Rpb24gdHJ5VG9FZGl0KHRhcmdldCwgcmVwbGFjZW1lbnQpIHsKICAgIHZhciB0YXJn ZXQgPSBkb2N1bWVudC5nZXRFbGVtZW50QnlJZCh0YXJnZXQpOwogICAgZG9jdW1lbnQuZ2V0U2Vs ZWN0aW9uKCkuc2VsZWN0QWxsQ2hpbGRyZW4oZG9jdW1lbnQuZ2V0RWxlbWVudEJ5SWQocmVwbGFj ZW1lbnQpKTsKICAgIHRhcmdldC5zdHlsZS5kaXNwbGF5ID0gIm5vbmUiOwogICAgZG9jdW1lbnQu ZXhlY0NvbW1hbmQoImluc2VydFRleHQiLCBmYWxzZSwgImFsbG93ZWQiKTsKICAgIHRhcmdldC5z dHlsZS5kaXNwbGF5ID0gIiI7Cn0KdHJ5VG9FZGl0KCJ0YXJnZXRBIiwgInRhcmdldEEiKTsKdHJ5 VG9FZGl0KCJ0YXJnZXRCIiwgInJlcGxhY2VtZW50QiIpOwo8L3NjcmlwdD4KPC9ib2R5Pgo8L2h0 bWw+