[macOS] Fix input[type=search] rendering in vertical writing mode #20146
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pxlcoder
added
the
Forms
|
EWS run on previous version of this PR (hash ae41bd8) |
nt1m
reviewed
Nov 8, 2023
Comment on lines
257
to
272
| auto effectiveAppearance = shadowHostStyle->effectiveAppearance(); | ||
| if (effectiveAppearance == StyleAppearance::TextField && shadowHostStyle->appearance() == StyleAppearance::TextField) { | ||
| auto elementStyle = resolveStyle(resolutionContext); | ||
| elementStyle.style->setDisplay(DisplayType::None); | ||
| return elementStyle; | ||
| } | ||
| if (appearance != StyleAppearance::SearchField) { | ||
| if (effectiveAppearance != StyleAppearance::SearchField && effectiveAppearance != StyleAppearance::TextField) { |
There was a problem hiding this comment.
These checks are not super pretty nor self explanatory IMO.
We should probably have a static method in this file that takes a RenderStyle and says whether the effectiveAppearance is default based on the writing mode and the effectiveAppearance.
There was a problem hiding this comment.
I'll add a static method with a good name for clarity. Though the method won't consider writing-mode, since that's a macOS specific behavior. It's more about the original appearance the author specified.
Comment on lines
+207
to
+212
| if (appearance == StyleAppearance::SearchField && searchFieldShouldAppearAsTextField(style)) { | ||
| appearance = StyleAppearance::TextField; | ||
| style.setEffectiveAppearance(appearance); | ||
| } |
There was a problem hiding this comment.
I somewhat wished we had all of this logic in autoAppearanceForElement
There was a problem hiding this comment.
I wanted to keep the special case separate, similar to the isControlStyled cases. Going to leave this as-is unless you feel strongly.
|
EWS run on previous version of this PR (hash 6bd2088)
|
pxlcoder
requested review from
nt1m,
hortont424,
whsieh,
rr-codes and
megangardner
nt1m
approved these changes
Nov 8, 2023
|
EWS run on current version of this PR (hash 2998648) |
pxlcoder
added
the
unsafe-merge-queue
https://bugs.webkit.org/show_bug.cgi?id=248334 rdar://102658361 Reviewed by Tim Nguyen. Native search fields are not height resizable, and have shadows, preventing their use in vertical writing mode. Instead, fall back to drawing native text fields, which are height resizable. Additionally, ensure that the cancel and results buttons are displayed correctly in vertical writing mode. * Source/WebCore/html/shadow/TextControlInnerElements.cpp: 253691@main introduced logic to hide the cancel and results buttons when authors specify `appearance: textfield` on search inputs. Preserve this functionality, but also allow cases where the user agent applies `appearance: textfield` for rendering purposes (in this case, in vertical writing mode), by comparing `appearance` in addition to `effectiveAppearance`. (WebCore::searchFieldStyleHasExplicitlySpecifiedTextFieldAppearance): (WebCore::SearchFieldResultsButtonElement::resolveCustomStyle): (WebCore::SearchFieldCancelButtonElement::resolveCustomStyle): * Source/WebCore/rendering/RenderTheme.cpp: (WebCore::RenderTheme::adjustStyle): Set an effective appearance of `TextField` for search fields using a vertical writing mode. * Source/WebCore/rendering/RenderTheme.h: (WebCore::RenderTheme::searchFieldShouldAppearAsTextField const): * Source/WebCore/rendering/RenderThemeMac.h: * Source/WebCore/rendering/RenderThemeMac.mm: (WebCore::RenderThemeMac::searchFieldShouldAppearAsTextField const): (WebCore::RenderThemeMac::adjustSearchFieldDecorationPartStyle const): While the results and cancel buttons are not being rotated, ensure there is not an excessive amount of padding at the logical top of the control in cases where the results button is hidden. Canonical link: https://commits.webkit.org/270415@main
webkit-commit-queue
force-pushed
the
eng/248334
branch
from
2998648
to
64d1b1b
Compare
|
Committed 270415@main (64d1b1b): https://commits.webkit.org/270415@main Reviewed commits have been landed. Closing PR #20146 and removing active labels. |
webkit-commit-queue
removed
the
unsafe-merge-queue
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
64d1b1b
2998648