Bug 256811 - textareas logical height with overflow auto shouldn't add scrollbar-thickness
Summary: textareas logical height with overflow auto shouldn't add scrollbar-thickness
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: Safari 16
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords: BrowserCompat, InRadar
Depends on:
Blocks:
 
Reported: 2023-05-15 15:18 PDT by Luke Warlow
Modified: 2023-05-19 08:01 PDT (History)
16 users (show)

See Also:


Attachments
Patch (2.21 KB, patch)
2023-05-17 19:58 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (82.95 KB, patch)
2023-05-18 08:24 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (49.96 KB, patch)
2023-05-18 08:38 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (50.06 KB, patch)
2023-05-19 06:39 PDT, zalan
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Luke Warlow 2023-05-15 15:18:20 PDT
Currently in WebKit if a textarea has overflow auto and overflow-wrap normal, it includes the scrollbar thickness in the logical height. This check doesn't exist in Firefox and no longer exists in Chrome as of https://chromium.googlesource.com/chromium/src/+/18481ffa332076e58fd9adbbea2062c2ba087dfb

It would be good to remove it from WebKit too for interop purposes.


if ((isHorizontalWritingMode() && (style().overflowX() == Overflow::Scroll ||  (style().overflowX() == Overflow::Auto && innerText->renderer()->style().overflowWrap() == OverflowWrap::Normal)))
    || (!isHorizontalWritingMode() && (style().overflowY() == Overflow::Scroll ||  (style().overflowY() == Overflow::Auto && innerText->renderer()->style().overflowWrap() == OverflowWrap::Normal))))
    logicalHeight += scrollbarThickness();

The above code inside of RenderTextControl.cpp would become something like the below code:

if ((isHorizontalWritingMode() && style().overflowX() == Overflow::Scroll)
    || (!isHorizontalWritingMode() && style().overflowY() == Overflow::Scroll))
    logicalHeight += scrollbarThickness();
Comment 1 Radar WebKit Bug Importer 2023-05-15 20:05:48 PDT
<rdar://problem/109384976>
Comment 2 zalan 2023-05-17 19:58:01 PDT
Created attachment 466393 [details]
Patch
Comment 3 zalan 2023-05-18 08:24:09 PDT
Created attachment 466398 [details]
Patch
Comment 4 zalan 2023-05-18 08:38:34 PDT
Created attachment 466399 [details]
Patch
Comment 5 Simon Fraser (smfr) 2023-05-18 14:05:09 PDT
Comment on attachment 466399 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=466399&action=review

> COMMIT_MESSAGE:7
> +Merged from Blink: https://chromium.googlesource.com/chromium/src/+/18481ffa332076e58fd9adbbea2062c2ba087dfb

I would like some more words here, explaining when we get a behavior change.

> COMMIT_MESSAGE:14
> +* LayoutTests/platform/mac/fast/forms/basic-textareas-expected.txt: WebKit matches Chrome now.

Don't care if it matches Chrome :)
Comment 6 zalan 2023-05-19 06:39:27 PDT
Created attachment 466416 [details]
Patch
Comment 7 EWS 2023-05-19 07:53:43 PDT
Committed 264251@main (8b48b8b9e8b8): <https://commits.webkit.org/264251@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 466416 [details].