Bug 24148 - REGRESSION(41206): Pixel test failures from positionForCoordinates patch need fixing.
Summary: REGRESSION(41206): Pixel test failures from positionForCoordinates patch need...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P1 Normal
Assignee: Ojan Vafai
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-02-24 18:07 PST by Ojan Vafai
Modified: 2009-02-26 13:17 PST (History)
2 users (show)

See Also:


Attachments
Fix test regressions from positionForCoordinates patch. (5.86 KB, patch)
2009-02-25 15:31 PST, Ojan Vafai
no flags Details | Formatted Diff | Diff
Fix test regressions from positionForCoordinates patch. 3 new results and 1 removal of an assert. (5.83 KB, patch)
2009-02-25 15:47 PST, Ojan Vafai
eric: review-
Details | Formatted Diff | Diff
Fix svg/custom/pointer-events-image.svg and svg/custom/pointer-events-text.svg (8.16 KB, patch)
2009-02-25 15:55 PST, Eric Seidel (no email)
hyatt: review+
Details | Formatted Diff | Diff
Fix test regressions from positionForCoordinates patch. 3 new results and 1 removal of an assert. (6.36 KB, patch)
2009-02-25 16:01 PST, Ojan Vafai
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ojan Vafai 2009-02-24 18:07:44 PST
I'm out of time for today and will get to these first thing tomorrow morning. Sorry about the regressions.

// Just needs new expected results.
fast/events/standalone-image-drag-to-editable.html	

// Hits a new assert I added. Will remove the assert and put a FIXME in it's place (the codepath wasn't new, just the assert)
fast/forms/search-click-in-placeholder.html

// Still need to look into these.
fast/inline/dirtyLinesForInline.html	
svg/custom/pointer-events-image.svg	
svg/custom/pointer-events-path.svg	
svg/custom/pointer-events-text.svg
Comment 2 Eric Seidel (no email) 2009-02-24 18:19:53 PST
For fast/forms/search-click-in-placeholder.html, the ASSERT it hits is:
http://trac.webkit.org/browser/trunk/WebCore/rendering/RenderBlock.cpp#L3464

Which should only be hit when there is a RootLineBox which has no children.  (Which makes no sense to me).  But it seems that is exactly the case with placeholder text?
Comment 3 Ojan Vafai 2009-02-25 15:31:37 PST
Created attachment 27990 [details]
Fix test regressions from positionForCoordinates patch.

 LayoutTests/ChangeLog                              |   12 ++++++++++++
 ...dalone-image-drag-to-editable-expected.checksum |    2 +-
 .../standalone-image-drag-to-editable-expected.png |  Bin 24155 -> 24164 bytes
 .../standalone-image-drag-to-editable-expected.txt |   11 +++++++----
 .../fast/inline/dirtyLinesForInline-expected.txt   |    2 +-
 .../svg/custom/pointer-events-path-expected.txt    |    2 +-
 WebCore/ChangeLog                                  |    9 +++++++++
 WebCore/rendering/RenderBlock.cpp                  |    3 ++-
 8 files changed, 33 insertions(+), 8 deletions(-)
Comment 4 Ojan Vafai 2009-02-25 15:47:12 PST
Created attachment 27993 [details]
Fix test regressions from positionForCoordinates patch. 3 new results and 1 removal of an assert.

 LayoutTests/ChangeLog                              |   12 ++++++++++++
 ...dalone-image-drag-to-editable-expected.checksum |    2 +-
 .../standalone-image-drag-to-editable-expected.png |  Bin 24155 -> 24164 bytes
 .../standalone-image-drag-to-editable-expected.txt |   11 +++++++----
 .../fast/inline/dirtyLinesForInline-expected.txt   |    2 +-
 .../svg/custom/pointer-events-path-expected.txt    |    2 +-
 WebCore/ChangeLog                                  |    9 +++++++++
 WebCore/rendering/RenderBlock.cpp                  |    3 ++-
 8 files changed, 33 insertions(+), 8 deletions(-)
Comment 5 Eric Seidel (no email) 2009-02-25 15:55:58 PST
Created attachment 27994 [details]
Fix svg/custom/pointer-events-image.svg and svg/custom/pointer-events-text.svg

 WebCore/ChangeLog                         |   28 +++++++++++++++++
 WebCore/rendering/RenderSVGInlineText.cpp |   18 +++++++----
 WebCore/rendering/SVGInlineTextBox.cpp    |   47 ++++++++++++++++-------------
 3 files changed, 66 insertions(+), 27 deletions(-)
Comment 6 Eric Seidel (no email) 2009-02-25 15:56:55 PST
Comment on attachment 27993 [details]
Fix test regressions from positionForCoordinates patch. 3 new results and 1 removal of an assert.

Needs documentation of the pointer-events-path improvement in the ChangeLOg.
Comment 7 Ojan Vafai 2009-02-25 16:01:03 PST
Created attachment 27995 [details]
Fix test regressions from positionForCoordinates patch. 3 new results and 1 removal of an assert.

 LayoutTests/ChangeLog                              |   23 ++++++++++++++++++++
 ...dalone-image-drag-to-editable-expected.checksum |    2 +-
 .../standalone-image-drag-to-editable-expected.png |  Bin 24155 -> 24164 bytes
 .../standalone-image-drag-to-editable-expected.txt |   11 ++++++---
 .../fast/inline/dirtyLinesForInline-expected.txt   |    2 +-
 .../svg/custom/pointer-events-path-expected.txt    |    2 +-
 WebCore/ChangeLog                                  |    9 +++++++
 WebCore/rendering/RenderBlock.cpp                  |    3 +-
 8 files changed, 44 insertions(+), 8 deletions(-)
Comment 8 Eric Seidel (no email) 2009-02-25 16:02:57 PST
Comment on attachment 27995 [details]
Fix test regressions from positionForCoordinates patch. 3 new results and 1 removal of an assert.

Looks good.
Comment 9 Eric Seidel (no email) 2009-02-25 16:36:04 PST
Comment on attachment 27995 [details]
Fix test regressions from positionForCoordinates patch. 3 new results and 1 removal of an assert.

Landed Ojan's patch as

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	M	LayoutTests/platform/mac/fast/events/standalone-image-drag-to-editable-expected.checksum
	M	LayoutTests/platform/mac/fast/events/standalone-image-drag-to-editable-expected.png
	M	LayoutTests/platform/mac/fast/events/standalone-image-drag-to-editable-expected.txt
	M	LayoutTests/platform/mac/fast/inline/dirtyLinesForInline-expected.txt
	M	LayoutTests/platform/mac/svg/custom/pointer-events-path-expected.txt
	M	WebCore/ChangeLog
	M	WebCore/rendering/RenderBlock.cpp
Committed r41237


Marking as obsolete so that Dave (or someone) can review my remaining patch on this bug. ;)
Comment 10 Dave Hyatt 2009-02-26 13:02:07 PST
Comment on attachment 27994 [details]
Fix svg/custom/pointer-events-image.svg and svg/custom/pointer-events-text.svg

r=me
Comment 11 Eric Seidel (no email) 2009-02-26 13:17:54 PST
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/rendering/RenderSVGInlineText.cpp
	M	WebCore/rendering/SVGInlineTextBox.cpp
Committed r41269

Yay!  No more regressions!