WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
123338
Re-enable simple line layout on non-Mac platforms
https://bugs.webkit.org/show_bug.cgi?id=123338
Summary
Re-enable simple line layout on non-Mac platforms
Antti Koivisto
Reported
2013-10-25 06:43:05 PDT
It hits ASSERT(run.charactersLength() >= run.length()) in textWidth(). for example:
http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug%20WK1/builds/4983
http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK1/builds/2321
Attachments
Patch
(1.61 KB, patch)
2014-02-06 08:39 PST
,
Csaba Osztrogonác
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Antti Koivisto
Comment 1
2013-10-25 06:48:37 PDT
Some
Antti Koivisto
Comment 2
2013-10-25 06:49:30 PDT
It is not obvious from code what the difference is between Mac and other platforms.
Zan Dobersek
Comment 3
2013-10-25 12:28:28 PDT
Non-Mac platforms don't define ENABLE_8BIT_TEXTRUN to 1, so the TextRun constructor implicitly converts the character data to a WTF::String. The length parameter that's passed to the TextRun constructor is then ignored, and the length of the complete string is used.
Zan Dobersek
Comment 4
2013-10-26 05:34:16 PDT
(In reply to
comment #3
)
> The length parameter that's passed to the TextRun constructor is then ignored, and the length of the complete string is used.
The length parameter is actually used as the xpos value rather than the length. Anyway, enabling the 8-bit text runs works for the GTK port. Modifying the TextRun construction in the textWidth function would likely work equally well. The 8-bit TextRun was introduced in
bug #96979
, but was only enabled for the Mac port because code of other platforms needed additional modifications to properly support the introduced optimization.
https://bugs.webkit.org/show_bug.cgi?id=96979#c9
The GTK port doesn't seem affected when running the tests with the support for 8-bit text runs enabled, so simply enabling the feature define would work. I'd also like to see people test out the feature on other ports and provide the proper changes so the define can be removed entirely.
Antti Koivisto
Comment 5
2013-10-28 07:54:50 PDT
Ah ok. Thanks for looking into this. We should really make String(const char*) explicit to avoid bugs like this.
Csaba Osztrogonác
Comment 6
2014-02-06 08:37:20 PST
ENABLE_8BIT_TEXTRUN guards were removed by
https://trac.webkit.org/changeset/163478
So let's go ahead.
Csaba Osztrogonác
Comment 7
2014-02-06 08:39:13 PST
Created
attachment 223334
[details]
Patch
Csaba Osztrogonác
Comment 8
2014-02-06 09:24:57 PST
Comment on
attachment 223334
[details]
Patch Clearing flags on attachment: 223334 Committed
r163537
: <
http://trac.webkit.org/changeset/163537
>
Csaba Osztrogonác
Comment 9
2014-02-06 09:25:03 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug