Bug 183079

Summary: [Simple line layout] Text with letter spacing is not positioned properly.
Product: WebKit Reporter: Tobi Reif <tobi>
Component: Layout and RenderingAssignee: zalan <zalan>
Status: RESOLVED FIXED    
Severity: Major CC: bfulgham, commit-queue, koivisto, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: Safari 11   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
the code
none
screenshot of the page in the zip, in Safari
none
screenshot of the page in the zip, in Chrome
none
r228947 centered text
none
screenshot_of webkit_228949
none
Test reduction
none
Patch none

Description Tobi Reif 2018-02-23 04:06:55 PST
Created attachment 334516 [details]
the code

(Please replace the title with something more specific.)

Please open the current version of https://tobireif.com/demos/grid/ in current Safari, set the window width to eg 630, and click on "Layout 2" -
the text "The Sane Choice!" is off-center and cut off.

(Same issue in the current Tech Preview.)

Please also resize the window to various widths and observe that line of text.

Attaching a zip so that you have the code that shows the Webkit issue even if I change the relevant code in my page.
Comment 1 Tobi Reif 2018-02-23 04:28:41 PST
P.S.

When I comment out this line it works:

  .layout2 .page aside {
    grid-row: 3;
    grid-column: 1;
    align-self: center;
    justify-self: center;
    /*
    text-align: center;
    */
  }

But I think it should work wit the code as it was before (as in the zip). (The version in the zip works in Chrome etc but not in Safari. I think that should get fixed in any case.)
Comment 2 Tobi Reif 2018-02-23 04:30:05 PST
"wit" -> "with"
Comment 3 Tobi Reif 2018-02-23 04:44:28 PST
Created attachment 334517 [details]
screenshot of the page in the zip, in Safari
Comment 4 Tobi Reif 2018-02-23 04:45:02 PST
Created attachment 334518 [details]
screenshot of the page in the zip, in Chrome
Comment 5 Tobi Reif 2018-02-23 04:45:36 PST
Perhaps the issue is related to justify-self:center?
Comment 6 zalan 2018-02-23 07:30:43 PST
It seems to be working fine with r228947. See attached screenshot. (unless I am not resizing the viewport right).
Comment 7 zalan 2018-02-23 07:31:09 PST
Created attachment 334531 [details]
r228947 centered text
Comment 8 Tobi Reif 2018-02-23 09:16:10 PST
Thanks for trying to reproduce the issue!

I tried it again and was able to reproduce the issue in the latest WebKit (as in current Safari and Tech Preview).

I downloaded the latest WebKit from
https://webkit.org/build-archives/
:
https://s3-us-west-2.amazonaws.com/minified-archives.webkit.org/mac-highsierra-x86_64-release/228949.zip

Then I used Chrome (with dev tools open so that Chrome show the dimensions) to get a window of 630 width.

Then I opened the page from the zip in the browser that's launched by run-webkit-archive, set the window width to the same width as the 630 wide Chrome window, and on the page clicked on the button "Layout 2".

The text "The Sane Choice!" is off-center and cut off, see
"screenshot_of webkit_228949.png".
Comment 9 Tobi Reif 2018-02-23 09:16:53 PST
Created attachment 334534 [details]
screenshot_of webkit_228949
Comment 10 Tobi Reif 2018-02-26 02:00:07 PST
I hope you can reproduce the issue using my latest steps.
Comment 11 Tobi Reif 2018-03-01 01:34:06 PST
I just tried it again, on a different machine, and was able to reproduce the issue again (using the attached zip named "the code").

I followed the latest steps (using Safari stable) I describe in comment 8 -
the text "The Sane Choice" is off-center and cut off (that is the issue).

Here's a screenshot:

https://app.crossbrowsertesting.com/public/ib8393c20a124bac/livetests/10895686/snapshots/z38c277928dc80b55a1d

I hope you can reproduce and fix the issue.
Comment 12 zalan 2018-03-01 14:21:29 PST
Simple and normal line layout disagree :(
Comment 13 zalan 2018-03-22 11:14:58 PDT
Created attachment 336291 [details]
Test reduction

Test reduction. Dynamic letter spacing fails with simple line layout.
Comment 14 Radar WebKit Bug Importer 2018-03-22 12:44:20 PDT
<rdar://problem/38762569>
Comment 15 zalan 2018-03-22 13:03:11 PDT
RenderText::m_canUseSimplifiedTextMeasuring needs updating after style change since we might be able to use the fast path after certain (letter spacing etc) style change.
Comment 16 zalan 2018-03-22 14:03:16 PDT
Created attachment 336310 [details]
Patch
Comment 17 WebKit Commit Bot 2018-03-22 14:59:46 PDT
Comment on attachment 336310 [details]
Patch

Clearing flags on attachment: 336310

Committed r229867: <https://trac.webkit.org/changeset/229867>
Comment 18 WebKit Commit Bot 2018-03-22 14:59:47 PDT
All reviewed patches have been landed.  Closing bug.
Comment 19 Tobi Reif 2018-03-23 02:41:50 PDT
Thanks all!
Comment 20 Tobi Reif 2018-03-27 00:31:50 PDT
I just checked in the latest WebKit from https://webkit.org/build-archives/ - it works! Thanks!
Comment 21 zalan 2018-03-27 07:15:31 PDT
(In reply to Tobi Reif from comment #20)
> I just checked in the latest WebKit from https://webkit.org/build-archives/
> - it works! Thanks!
Thank you for verifying it!