Bug 125278 - Save original text for RenderText to a map
Summary: Save original text for RenderText to a map
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-12-04 19:42 PST by Antti Koivisto
Modified: 2013-12-06 16:32 PST (History)
6 users (show)

See Also:

Attachments
patch (8.70 KB, patch)
2013-12-05 12:21 PST, Antti Koivisto 		darin: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion (1.21 MB, application/zip)
2013-12-05 13:03 PST, Build Bot 		no flags Details
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (1.01 MB, application/zip)
2013-12-05 13:16 PST, Build Bot 		no flags Details
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 Antti Koivisto 2013-12-04 19:42:18 PST 
foo
Comment 1 Antti Koivisto 2013-12-05 12:21:00 PST 
Created attachment 218529 [details]
patch
Comment 2 Darin Adler 2013-12-05 12:32:22 PST 
Comment on attachment 218529 [details]
patch

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

Gotta fix the NDEBUG problem. Otherwise looks good to me, and I can’t spot any bugs. Not sure we have sufficient test coverage.

> Source/WebCore/rendering/RenderText.cpp:100
> +typedef HashMap<const RenderText*, String> OriginalTextMap;

Not sure we benefit from using a typedef for this since the type is only used twice. Also extra blank line before this typedef.

> Source/WebCore/rendering/RenderText.cpp:204
>  RenderText::~RenderText()
>  {
> +    if (m_originalTextDiffersFromRendered)
> +        originalTextMap().remove(this);
>  }

We need to take this out of #ifndef NDEBUG since we are going to use it for non-debugging purposes.
Comment 3 Antti Koivisto 2013-12-05 12:40:57 PST 
> We need to take this out of #ifndef NDEBUG since we are going to use it for non-debugging purposes.

oops!
Comment 4 Build Bot 2013-12-05 13:03:19 PST 
Comment on attachment 218529 [details]
patch

Attachment 218529 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/44178030

New failing tests:
svg/text/text-ws-02-t.svg
svg/W3C-SVG-1.1-SE/text-tspan-02-b.svg
svg/W3C-SVG-1.1/text-ws-02-t.svg
svg/custom/text-whitespace-handling.svg
Comment 5 Build Bot 2013-12-05 13:03:21 PST 
Created attachment 218536 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 6 Build Bot 2013-12-05 13:16:49 PST 
Comment on attachment 218529 [details]
patch

Attachment 218529 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/44248028

New failing tests:
svg/text/text-ws-02-t.svg
svg/W3C-SVG-1.1-SE/text-tspan-02-b.svg
svg/W3C-SVG-1.1/text-ws-02-t.svg
svg/custom/text-whitespace-handling.svg
Comment 7 Build Bot 2013-12-05 13:16:51 PST 
Created attachment 218539 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 8 Antti Koivisto 2013-12-06 16:32:10 PST 
https://trac.webkit.org/r160259 (with fixes)