Fix text input issue in shadow DOM with delegatesFocus [chromium/src : main]
Perry (Gerrit)
New activity on the change![]( "Open in Gerrit")
Related details
- Mason Freed
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mason Freed (Gerrit)
Mason Freed added 3 comments![]( "Open in Gerrit")
The code here looks good. The only thing missing, given that the last change causes a web-exposed breakage here, is to add a feature flag that enables the new behavior. This can be set to "stable" by default - it is just to be used as a "kill switch". LMK if you need pointers for how to do that.
selection_length = GetElement()
.GetDocument()
.GetFrame()
->Selection()Is it worth getting the selection once, and using it in both places?
assert_equals(input.value, 'FOOFOO', "keyboard typing will update the input value.");You confirmed that this fails before this patch?
Related details
- Perry
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Perry (Gerrit)
Perry added 3 comments![]( "Open in Gerrit")
The code here looks good. The only thing missing, given that the last change causes a web-exposed breakage here, is to add a feature flag that enables the new behavior. This can be set to "stable" by default - it is just to be used as a "kill switch". LMK if you need pointers for how to do that.
Done
selection_length = GetElement()
.GetDocument()
.GetFrame()
->Selection()Is it worth getting the selection once, and using it in both places?
Done
assert_equals(input.value, 'FOOFOO', "keyboard typing will update the input value.");You confirmed that this fails before this patch?
Yes. I reverted this CL and only kept the test change on patchset 6. Tryjobs run failed.
https://chromium-review.googlesource.com/c/chromium/src/+/6918543/6?tab=checks
Related details
- Mason Freed
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mason Freed (Gerrit)
Mason Freed added 4 comments![]( "Open in Gerrit")
if (!CanEdit())
return false;Does this need to stay, if the feature is disabled?
// Killswitch for https://crbug.com/400317114 fix.Can you add that this lands in M142, and the flag can be removed in M144?
assert_equals(input.value, 'FOOFOO', "keyboard typing will update the input value.");PerryYou confirmed that this fails before this patch?
Yes. I reverted this CL and only kept the test change on patchset 6. Tryjobs run failed.
https://chromium-review.googlesource.com/c/chromium/src/+/6918543/6?tab=checks
- Perry
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Perry (Gerrit)
Perry added 2 comments![]( "Open in Gerrit")
if (!CanEdit())
return false;Does this need to stay, if the feature is disabled?
If this feature is disabled, the code on line 112 is equivalent here, the only difference being that the judgment condition has been moved to after obtaining focusd element, but I don't think this affects the code logic.
Can you add that this lands in M142, and the flag can be removed in M144?
- Mason Freed
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mason Freed (Gerrit)
Mason Freed voted and added 2 comments![]( "Open in Gerrit")
Votes added by Mason Freed
| Code-Review | +1 |
2 comments
LGTM! Thanks! Let me know if you need me to land this.
if (!CanEdit())
return false;PerryDoes this need to stay, if the feature is disabled?
If this feature is disabled, the code on line 112 is equivalent here, the only difference being that the judgment condition has been moved to after obtaining focusd element, but I don't think this affects the code logic.
Yeah fair. I guess nothing can change between those. Ok, thanks.
Related details
- Perry
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Blink W3C Test Autoroller (Gerrit)
Message from Blink W3C Test Autoroller![]( "Open in Gerrit")
Exportable changes to web-platform-tests were detected in this CL and a pull request in the upstream repo has been made: https://github.com/web-platform-tests/wpt/pull/54910.
When this CL lands, the bot will automatically merge the PR on GitHub if the required GitHub checks pass; otherwise, ecosystem-infra@ team will triage the failures and may contact you.
WPT Export docs:
https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md#Automatic-export-process
Related details
- Perry
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Perry (Gerrit)
Perry voted Commit-Queue+2![]( "Open in Gerrit")
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Fix text input issue in shadow DOM with delegatesFocus
If the focusable area is a text control element, but the selection is
empty or on non-editable elements, then the focused text control cannot
input text. This CL adds a check whether the focused element is a text
control, based on checking whether the selection can edit.
- M third_party/blink/renderer/core/editing/editor_key_bindings.cc
- M third_party/blink/renderer/core/html/forms/text_field_input_type.cc
- M third_party/blink/renderer/platform/runtime_enabled_features.json5
- M third_party/blink/web_tests/external/wpt/shadow-dom/focus/text-selection-with-delegatesFocus-text-control.html
Code-Review: +1 by Mason Freed
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Blink W3C Test Autoroller (Gerrit)
Message from Blink W3C Test Autoroller![]( "Open in Gerrit")
The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/54910
Related details
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |