| Summary: | Complex text handling should opt out of bounded text layout. | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | zalan <zalan> | ||||||||||
| Component: | Layout and Rendering | Assignee: | zalan <zalan> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | bfulgham, commit-queue, darin, dbates, ews-watchlist, jbedard, mmaxfield, ryanhaddad, simon.fraser, thorton, webkit-bug-importer, zalan | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=202054 | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
zalan
2018-06-12 11:47:10 PDT
Created attachment 342573 [details]
Patch
Comment on attachment 342573 [details] Patch Clearing flags on attachment: 342573 Committed r232774: <https://trac.webkit.org/changeset/232774> All reviewed patches have been landed. Closing bug. Reverted r232774 for reason: Breaks internal builds. Committed r232784: <https://trac.webkit.org/changeset/232784> Created attachment 342621 [details]
Patch
Comment on attachment 342621 [details] Patch Clearing flags on attachment: 342621 Committed r232787: <https://trac.webkit.org/changeset/232787> All reviewed patches have been landed. Closing bug. Comment on attachment 342621 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=342621&action=review > Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm:147 > +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED == 101400) || (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED == 120000) How can == be right here? Surely it should be >= instead. I think need to update CoreTextSPI.h so people without access to CoreTextPriv.h can build on macOS Mojave and newer. Also, I wasn’t able to build locally; maybe that means I need an updated version of the Apple internal headers. I didn’t see any announcement about that for Apple engineers. Reopening to attach new patch. Created attachment 342829 [details]
Patch
A few notes: Darin is right, we need to update CoreTextSPI. This will also break building on Seed 1, I'm verifying this fix with Seed 2. I talked with Zalan this morning, the '==' is right, at least for the time being. See Ryan's rollout. Comment on attachment 342829 [details]
Patch
Verified this on Seed 2
Comment on attachment 342829 [details] Patch Attachment 342829 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/8201362 New failing tests: http/tests/preload/onload_event.html Created attachment 342838 [details]
Archive of layout-test-results from ews205 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews205 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment on attachment 342829 [details]
Patch
Tree is red for Windows tests, landing.
Comment on attachment 342829 [details] Patch Clearing flags on attachment: 342829 Committed r232894: <https://trac.webkit.org/changeset/232894> All reviewed patches have been landed. Closing bug. (In reply to Jonathan Bedard from comment #12) > I talked with Zalan this morning, the '==' is right, at least for the time being. Are you or Zalan able to explain why "==" is correct? (In reply to Daniel Bates from comment #19) > (In reply to Jonathan Bedard from comment #12) > > I talked with Zalan this morning, the '==' is right, at least for the time being. > > Are you or Zalan able to explain why "==" is correct? It had to do with which SDK the SPI was available...been more than a year since I looked at it, though. With deprecation of the iOS 12 builders, and the fact that we are only supporting the Mojave G release now, this code should be dead. |