Bug 104643 – Mail hangs when resizing the font size of a large RTL text

• Component: HTML Editing
• Status: RESOLVED FIXED

[Ryosuke Niwa]

Mail hangs when resizing the font size of a large RTL text

[Ryosuke Niwa]

<rdar://problem/11824641>

[Ryosuke Niwa]

Created attachment 178742 [details]
Fixes the bug

[Ryosuke Niwa]

Created attachment 178746 [details]
Resolved conflicts

[Build Bot]

Comment on attachment 178746 [details]
Resolved conflicts

Attachment 178746 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15231924

[Ryosuke Niwa]

Comment on attachment 178746 [details]
Resolved conflicts

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

> Source/WebCore/editing/ApplyStyleCommand.cpp:826
>      ASSERT(runStart && runEnd && runStart->parentNode() == runEnd->parentNode());

Oh oops, I need to modify this assertion and add a similar one on L841. I'll do that once it's been reviewed.

10>c:\cygwin\home\buildbot\WebKit\Source\WebCore\editing\ApplyStyleCommand.cpp(826) : error C2065: 'runEnd' : undeclared identifier
10>c:\cygwin\home\buildbot\WebKit\Source\WebCore\editing\ApplyStyleCommand.cpp(826) : error C2227: left of '->parentNode' must point to class/struct/union/generic type
10>        type is ''unknown-type''

[Ryosuke Niwa]

Comment on attachment 178746 [details]
Resolved conflicts

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

> Source/WebCore/editing/ApplyStyleCommand.cpp:782
> -        RefPtr<Node> runStart = node;
> -        RefPtr<Node> runEnd = node;
> +        Node* runStart = node.get();
> +        Node* runEnd = node.get();

Note that the code below no longer modifies DOM so we don't need a RefPtr anymore.

[Enrica Casucci]

Comment on attachment 178746 [details]
Resolved conflicts

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

Looks good to me. Only few nit picks. Please fix the ASSERT problem before landing

> Source/WebCore/ChangeLog:19
> +        (InlineRunToApplyStyle):

Please remove

> Source/WebCore/ChangeLog:21
> +        (WebCore):

Please remove

> Source/WebCore/ChangeLog:29
> +        (ApplyStyleCommand):

Ditto

> Source/WebCore/ChangeLog:32
> +        (StyleChange):

Ditto

> Source/WebCore/editing/ApplyStyleCommand.cpp:719
> +struct InlineRunToApplyStyle {

Why did you choose a struct instead of a class?

[Ryosuke Niwa]

Comment on attachment 178746 [details]
Resolved conflicts

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

>> Source/WebCore/editing/ApplyStyleCommand.cpp:719
>> +struct InlineRunToApplyStyle {
> 
> Why did you choose a struct instead of a class?

Because all members of this class is public and it doesn't have any getters or setters.

[Ryosuke Niwa]

Committed r137370: <http://trac.webkit.org/changeset/137370>