Bug 150986 - Some style changes cause tatechuyoko to be drawn off center
Summary: Some style changes cause tatechuyoko to be drawn off center
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on: 151186
Blocks:
  Show dependency tree / graph
 
Reported: 2015-11-06 17:08 PST by Myles C. Maxfield
Modified: 2015-11-12 03:47 PST (History)
10 users (show)

See Also:

Attachments
Patch (5.21 KB, patch)
2015-11-06 17:13 PST, Myles C. Maxfield 		darin: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2015-11-06 17:08:29 PST 
Some style changes cause tatechuyoko to be drawn off center
Comment 1 Myles C. Maxfield 2015-11-06 17:13:30 PST 
Created attachment 264983 [details]
Patch
Comment 2 Myles C. Maxfield 2015-11-06 17:14:25 PST 
<rdar://problem/20748013>
Comment 3 Darin Adler 2015-11-07 16:31:40 PST 
Comment on attachment 264983 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=264983&action=review

> Source/WebCore/rendering/RenderCombineText.cpp:93
> +    // An ancestor element may trigger us to relayout even when we're already combined.

We should use better grammar in this comment. I don’t think “trigger us to relayout” is grammatical.

> Source/WebCore/rendering/RenderCombineText.cpp:149
> +        RenderText::setRenderedText(objectReplacementCharacterString.get().impl());

The .impl() here is really mysterious. Why would we need to include that? The function takes a const String&, not a StringImpl*. I think that saying impl() just introduces another String temporary and thus reference count churn.
Comment 4 Myles C. Maxfield 2015-11-09 11:40:46 PST 
Committed r192169: <http://trac.webkit.org/changeset/192169>