Bug 191630

Summary: REGRESSION(r237845): [cairo] Hyperlink underscore layout issue
Product: WebKit Reporter: Jim Mason <jmason>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, cgarcia, dbates, mcatanzaro, mmaxfield
Priority: P2 Keywords: Regression
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=191239
Bug Depends on: 191239    
Bug Blocks:    
Attachments:
Description Flags
Screenshot 2.23.0 (layout problem)
none
Screenshot 2.22.3 (layout OK)
none
Patch mcatanzaro: review+

Jim Mason
Reported 2018-11-14 03:30:01 PST
Created attachment 354792 [details] Screenshot 2.23.0 (layout problem) In builds from svn, the hyperlink underscore is very close to the text, such that it is obscured by the text. Attached are screenshots under 2.23.0 (layout issue) and under 2.22.3 (layout OK). Both are under epiphany 3.30.2. The 'Use system fonts' preference has no effect, nor does zooming in or out.
Attachments
Screenshot 2.23.0 (layout problem) (876.87 KB, image/png)
2018-11-14 03:30 PST, Jim Mason
no flags
Screenshot 2.22.3 (layout OK) (873.92 KB, image/png)
2018-11-14 03:30 PST, Jim Mason
no flags
Patch (1.42 KB, patch)
2018-11-20 02:49 PST, Carlos Garcia Campos
mcatanzaro: review+
Jim Mason
Comment 1 2018-11-14 03:30:58 PST
Created attachment 354793 [details] Screenshot 2.22.3 (layout OK)
Michael Catanzaro
Comment 2 2018-11-16 09:46:14 PST
Myles you broke hyperlinks aaaah!
Myles C. Maxfield
Comment 3 2018-11-16 11:13:40 PST
Uh oh, there must have been some subtle difference between the Mac version and the GTK version. From looking at the screenshots it looks like the coordinate system for calculating the intersection positions is wrong. It's difficult for me to debug GTK since I don't have a machine... I'd be happy to work with someone who has a GTK machine to try to figure this out.
Michael Catanzaro
Comment 4 2018-11-19 08:02:36 PST
From bug #191239: >> Source/WebCore/platform/graphics/FontCascade.cpp:1751 >> + , m_translation(AffineTransform::translation(textOrigin.x(), textOrigin.y()).scale(1, -1)) > > This is what broke cairo ports, we didn't have that scale(1, -1) in the cairo impl.
Carlos Garcia Campos
Comment 5 2018-11-20 02:49:31 PST
Michael Catanzaro
Comment 6 2018-11-20 07:14:42 PST
Comment on attachment 355329 [details] Patch Well it's not a good solution, but this is a release blocker and it would be nice to release. Maybe Myles would know the right approach.
Myles C. Maxfield
Comment 7 2018-11-20 12:11:38 PST
(In reply to Michael Catanzaro from comment #6) > Comment on attachment 355329 [details] > Patch > > Well it's not a good solution, but this is a release blocker and it would be > nice to release. > > Maybe Myles would know the right approach. I think this solution is fine. Sorry for missing that scale :(
Michael Catanzaro
Comment 8 2018-11-20 15:46:51 PST
(In reply to Myles C. Maxfield from comment #7) > I think this solution is fine. Why does only one port need the vertical flip? And in cross-platform code?
Carlos Garcia Campos
Comment 9 2018-11-21 01:11:31 PST
Jim Mason
Comment 10 2018-11-21 06:07:16 PST
Thank you! Fix verified in build from SVN source 2018-11-21 @ 1200WET
Note You need to log in before you can comment on or make changes to this bug.