The x-position of the caret in an empty text field does not match the x-position of the caret in a text field. You can see this for yourself by visiting <data:text/html,<input placeholder="text"><textarea placeholder="text"></textarea>>.
<rdar://problem/47161070>
Created attachment 363140 [details] Work-in-progress
Created attachment 363348 [details] Patch and layout test
Comment on attachment 363348 [details] Patch and layout test Attachment 363348 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/11334077 New failing tests: fast/forms/textarea-scrolled-type.html fast/forms/textarea-width.html fast/forms/textarea-placeholder-visibility-2.html fast/forms/textarea-align.html fast/forms/form-element-geometry.html fast/forms/text-control-intrinsic-widths.html fast/forms/textarea-scroll-height.html fast/forms/textarea-scrollbar.html fast/forms/negativeLineHeight.html fast/forms/textarea-setinnerhtml.html fast/forms/textarea-placeholder-visibility-1.html fast/forms/textAreaLineHeight.html fast/forms/textarea-placeholder-pseudo-style.html fast/forms/basic-textareas.html
Created attachment 363368 [details] Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Created attachment 363370 [details] Patch and layout tests
Comment on attachment 363370 [details] Patch and layout tests View in context: https://bugs.webkit.org/attachment.cgi?id=363370&action=review > Source/WebCore/css/html.css:400 > + padding: 0.2em 0.5em 0.3em 0.5em; /* Keep padding-left, padding-right in sync with textarea selector (below). */ Should say "keep the values the same", not "keep them in sync" since "in sync" sounds like something more sophisticated that could mean something different. Also, this comment, like all comments, ought to say *why*, not just leave instructions for future programmers to follow without rationale. Still want to keep the comment wording short, if possible. > Source/WebCore/css/html.css:681 > + /* Keep padding-left, padding-right sync with input selector (above). */ Would be best if the comment format was identical to above. The other one is at the end of a line but this is between lines. Also, this says "keep sync" rather than "keep in sync", but should be reworded anyway. But also, the thing above isn’t the input selector. On the iOS Family platform, which is the only platform this code is active on, the above selector is a selector that matches *many* things, including textarea, just like this one. So it’s a misnomer to call the thing about the "input selector". And that observation points to a way to avoid this problem. Instead of "padding: 2x", which I suppose is what we want for other platforms, on the iOS Family platforms maybe we should instead just do "padding-top: 2px; padding-bottom: 2px", then we will inherit the padding values from the earlier rule and won’t need padding-left/padding-right values nor a comment telling us to keep things "in sync". Still might need a comment explaining why we carefully avoid setting padding-left and padding-right.
(In reply to Darin Adler from comment #7) > Comment on attachment 363370 [details] > Patch and layout tests > > View in context: > https://bugs.webkit.org/attachment.cgi?id=363370&action=review > > > Source/WebCore/css/html.css:400 > > + padding: 0.2em 0.5em 0.3em 0.5em; /* Keep padding-left, padding-right in sync with textarea selector (below). */ > > Should say "keep the values the same", not "keep them in sync" since "in > sync" sounds like something more sophisticated that could mean something > different. Also, this comment, like all comments, ought to say *why*, not > just leave instructions for future programmers to follow without rationale. > Still want to keep the comment wording short, if possible. > > > Source/WebCore/css/html.css:681 > > + /* Keep padding-left, padding-right sync with input selector (above). */ > > Would be best if the comment format was identical to above. The other one is > at the end of a line but this is between lines. Also, this says "keep sync" > rather than "keep in sync", but should be reworded anyway. > Will fix comment wording. > But also, the thing above isn’t the input selector. On the iOS Family > platform, which is the only platform this code is active on, the above > selector is a selector that matches *many* things, including textarea, just > like this one. So it’s a misnomer to call the thing about the "input > selector". And that observation points to a way to avoid this problem. > Instead of "padding: 2x", which I suppose is what we want for other > platforms, on the iOS Family platforms maybe we should instead just do > "padding-top: 2px; padding-bottom: 2px", then we will inherit the padding > values from the earlier rule and won’t need padding-left/padding-right > values nor a comment telling us to keep things "in sync". Still might need a > comment explaining why we carefully avoid setting padding-left and > padding-right. Yeah, that's a better idea. Will do that.
Comment on attachment 363370 [details] Patch and layout tests View in context: https://bugs.webkit.org/attachment.cgi?id=363370&action=review >>> Source/WebCore/css/html.css:400 >>> + padding: 0.2em 0.5em 0.3em 0.5em; /* Keep padding-left, padding-right in sync with textarea selector (below). */ >> >> Should say "keep the values the same", not "keep them in sync" since "in sync" sounds like something more sophisticated that could mean something different. Also, this comment, like all comments, ought to say *why*, not just leave instructions for future programmers to follow without rationale. Still want to keep the comment wording short, if possible. > > Will fix comment wording. Removed this comment entirely. Instead added the following comment below: /* On iOS we want to inherit the left and right padding for consistency with other form controls (e.g. <input type="text">). We want to use override the top and bottom padding to ensure we have a symmetric look for text areas. */
Created attachment 363544 [details] To land
Comment on attachment 363544 [details] To land Clearing flags on attachment: 363544 Committed r242379: <https://trac.webkit.org/changeset/242379>
All reviewed patches have been landed. Closing bug.
(In reply to Daniel Bates from comment #9) > /* On iOS we want to inherit the left and right padding for consistency with > other form controls (e.g. <input type="text">). We want to use override the > top and bottom padding to ensure we have a symmetric look for text areas. */ The phrase "use override" might need to be revised; doesn't seem quite right.
(In reply to Darin Adler from comment #13) > (In reply to Daniel Bates from comment #9) > > /* On iOS we want to inherit the left and right padding for consistency with > > other form controls (e.g. <input type="text">). We want to use override the > > top and bottom padding to ensure we have a symmetric look for text areas. */ > > The phrase "use override" might need to be revised; doesn't seem quite right. Yikes! Changing "use override" to "override"
(In reply to Daniel Bates from comment #14) > (In reply to Darin Adler from comment #13) > > (In reply to Daniel Bates from comment #9) > > > /* On iOS we want to inherit the left and right padding for consistency with > > > other form controls (e.g. <input type="text">). We want to use override the > > > top and bottom padding to ensure we have a symmetric look for text areas. */ > > > > The phrase "use override" might need to be revised; doesn't seem quite right. > > Yikes! Changing "use override" to "override" Fixed in <https://trac.webkit.org/r242389>.
It appears that the changes in https://trac.webkit.org/changeset/242379/webkit caused an API failure on iOS: Failed TestWebKitAPI.DragAndDropTests.ContentEditableToTextarea 2019-03-04 15:04:34.682 TestWebKitAPI[44455:23074074] Expected selection rects: ( "NSRect: {{6, 203}, {990, 232}}" ) but observed: ( "NSRect: {{101, 203}, {990, 232}}" ) /Volumes/Data/slave/ios-simulator-12-release/build/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm:137 Value of: [expected isEqualToArray:observed] Actual: false Expected: true Build: https://build.webkit.org/builders/Apple%20iOS%2012%20Simulator%20Release%20WK2%20%28Tests%29/builds/2957 reproduced this locally and found 242379 causes the failure while 242378 does not.
(In reply to Truitt Savell from comment #16) > It appears that the changes in > https://trac.webkit.org/changeset/242379/webkit > > caused an API failure on iOS: > Failed > > TestWebKitAPI.DragAndDropTests.ContentEditableToTextarea > 2019-03-04 15:04:34.682 TestWebKitAPI[44455:23074074] Expected > selection rects: ( > "NSRect: {{6, 203}, {990, 232}}" > ) but observed: ( > "NSRect: {{101, 203}, {990, 232}}" > ) > > > /Volumes/Data/slave/ios-simulator-12-release/build/Tools/TestWebKitAPI/Tests/ > ios/DragAndDropTestsIOS.mm:137 > Value of: [expected isEqualToArray:observed] > Actual: false > Expected: true > > Build: > https://build.webkit.org/builders/ > Apple%20iOS%2012%20Simulator%20Release%20WK2%20%28Tests%29/builds/2957 > > reproduced this locally and found 242379 causes the failure while 242378 > does not. Will look.
(In reply to Daniel Bates from comment #17) > (In reply to Truitt Savell from comment #16) > > It appears that the changes in > > https://trac.webkit.org/changeset/242379/webkit > > > > caused an API failure on iOS: > > Failed > > > > TestWebKitAPI.DragAndDropTests.ContentEditableToTextarea > > 2019-03-04 15:04:34.682 TestWebKitAPI[44455:23074074] Expected > > selection rects: ( > > "NSRect: {{6, 203}, {990, 232}}" > > ) but observed: ( > > "NSRect: {{101, 203}, {990, 232}}" > > ) > > > > > > /Volumes/Data/slave/ios-simulator-12-release/build/Tools/TestWebKitAPI/Tests/ > > ios/DragAndDropTestsIOS.mm:137 > > Value of: [expected isEqualToArray:observed] > > Actual: false > > Expected: true > > > > Build: > > https://build.webkit.org/builders/ > > Apple%20iOS%2012%20Simulator%20Release%20WK2%20%28Tests%29/builds/2957 > > > > reproduced this locally and found 242379 causes the failure while 242378 > > does not. > > Will look. This result is expected now that the left padding of a textarea is specified in ems (so, dependent on the font size). Committed updated test expectations in <https://trac.webkit.org/changeset/242530>.
(In reply to Truitt Savell from comment #16) > It appears that the changes in > https://trac.webkit.org/changeset/242379/webkit > > caused an API failure on iOS: > Failed > > TestWebKitAPI.DragAndDropTests.ContentEditableToTextarea > 2019-03-04 15:04:34.682 TestWebKitAPI[44455:23074074] Expected > selection rects: ( > "NSRect: {{6, 203}, {990, 232}}" > ) but observed: ( > "NSRect: {{101, 203}, {990, 232}}" > ) > > > /Volumes/Data/slave/ios-simulator-12-release/build/Tools/TestWebKitAPI/Tests/ > ios/DragAndDropTestsIOS.mm:137 > Value of: [expected isEqualToArray:observed] > Actual: false > Expected: true > > Build: > https://build.webkit.org/builders/ > Apple%20iOS%2012%20Simulator%20Release%20WK2%20%28Tests%29/builds/2957 > > reproduced this locally and found 242379 causes the failure while 242378 > does not. By the way, I assumed you made typos and meant r242389 and r242388.