Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Toggling the spellcheck attribute on input elements doesn't toggle spelling markers #21637

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

pxlcoder
Copy link
Member

@pxlcoder pxlcoder commented Dec 11, 2023

971fabe

Toggling the spellcheck attribute on input elements doesn't toggle spelling markers
https://bugs.webkit.org/show_bug.cgi?id=265958
rdar://119269616

Reviewed by Wenson Hsieh.

Add or remove spelling and grammar markers from <input> and <textarea> if the
control has been focused at some point, and the spellcheck attribute is toggled.

General `contenteditable` are currently excluded from this behavior, since
storing additional information about whether an element has ever been focused
would be expensive. Additionally, the behavior this patch implements matches
Firefox exactly.

* LayoutTests/editing/spelling/spellcheck-attribute-toggle-expected.html: Added.
* LayoutTests/editing/spelling/spellcheck-attribute-toggle.html: Added.
* Source/WebCore/dom/Document.h:
(WebCore::Document::hasEverHadSelectionInsideTextFormControl const):
(WebCore::Document::setHasEverHadSelectionInsideTextFormControl):

Store a bit to indicate if a selection has ever been inside a text form control.
Since spellchecking can only occur if a selection has been made, this serves to
provide an optimization where changes to the spellcheck attribute do not perform
unnecessary tree walks when the page is first loaded, before any editing commands
have occured, and before a selection is made.

* Source/WebCore/html/HTMLElement.cpp:
(WebCore::HTMLElement::attributeChanged):
(WebCore::HTMLElement::effectiveSpellcheckAttributeChanged):

The effective value of the spellcheck attribute is determined by the nearest
ancestor if the element itself does not have the attribute specified. Perform
a descendant tree walk to find affected elements for a given attribute change.
Note the optimization described earlier to avoid unnecessary tree walks.

* Source/WebCore/html/HTMLElement.h:
* Source/WebCore/html/HTMLTextFormControlElement.cpp:
(WebCore::HTMLTextFormControlElement::selectionChanged):
(WebCore::HTMLTextFormControlElement::effectiveSpellcheckAttributeChanged):

Toggle markers only if the control has had a selection at some point.

* Source/WebCore/html/HTMLTextFormControlElement.h:

Canonical link: https://commits.webkit.org/271927@main

e1f0b02

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ api-wpe
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 βœ… πŸ›  gtk
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ gtk-wk2
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-wk2-stress
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@pxlcoder pxlcoder self-assigned this Dec 11, 2023
@pxlcoder pxlcoder added the Forms For bugs specific to form elements (checkboxes, buttons, text fields, etc.) label Dec 11, 2023
@pxlcoder
Copy link
Member Author

Thanks for the review!

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 11, 2023
@pxlcoder pxlcoder removed the merging-blocked Applied to prevent a change from being merged label Dec 12, 2023
@pxlcoder pxlcoder added the merge-queue Applied to send a pull request to merge-queue label Dec 12, 2023
…elling markers

https://bugs.webkit.org/show_bug.cgi?id=265958
rdar://119269616

Reviewed by Wenson Hsieh.

Add or remove spelling and grammar markers from <input> and <textarea> if the
control has been focused at some point, and the spellcheck attribute is toggled.

General `contenteditable` are currently excluded from this behavior, since
storing additional information about whether an element has ever been focused
would be expensive. Additionally, the behavior this patch implements matches
Firefox exactly.

* LayoutTests/editing/spelling/spellcheck-attribute-toggle-expected.html: Added.
* LayoutTests/editing/spelling/spellcheck-attribute-toggle.html: Added.
* Source/WebCore/dom/Document.h:
(WebCore::Document::hasEverHadSelectionInsideTextFormControl const):
(WebCore::Document::setHasEverHadSelectionInsideTextFormControl):

Store a bit to indicate if a selection has ever been inside a text form control.
Since spellchecking can only occur if a selection has been made, this serves to
provide an optimization where changes to the spellcheck attribute do not perform
unnecessary tree walks when the page is first loaded, before any editing commands
have occured, and before a selection is made.

* Source/WebCore/html/HTMLElement.cpp:
(WebCore::HTMLElement::attributeChanged):
(WebCore::HTMLElement::effectiveSpellcheckAttributeChanged):

The effective value of the spellcheck attribute is determined by the nearest
ancestor if the element itself does not have the attribute specified. Perform
a descendant tree walk to find affected elements for a given attribute change.
Note the optimization described earlier to avoid unnecessary tree walks.

* Source/WebCore/html/HTMLElement.h:
* Source/WebCore/html/HTMLTextFormControlElement.cpp:
(WebCore::HTMLTextFormControlElement::selectionChanged):
(WebCore::HTMLTextFormControlElement::effectiveSpellcheckAttributeChanged):

Toggle markers only if the control has had a selection at some point.

* Source/WebCore/html/HTMLTextFormControlElement.h:

Canonical link: https://commits.webkit.org/271927@main
@webkit-commit-queue
Copy link
Collaborator

Committed 271927@main (971fabe): https://commits.webkit.org/271927@main

Reviewed commits have been landed. Closing PR #21637 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 971fabe into WebKit:main Dec 12, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Forms For bugs specific to form elements (checkboxes, buttons, text fields, etc.)
Projects
None yet
5 participants