Bug 80564

Created attachment 130757 [details] Patch

Comment on attachment 130757 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130757&action=review The change looks OK, but I'll delegate the review to another reviewer since I am not familiar with the rendering text. > Source/WebCore/ChangeLog:11 > + No behavior change. Isn't there any existing test that _can_ be affected by this change, if the change were wrong? I wanted to see "Test: xxx.html (No change behavior)", to confirm that the change is correct.

Comment on attachment 130757 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130757&action=review >> Source/WebCore/ChangeLog:11 >> + No behavior change. > > Isn't there any existing test that _can_ be affected by this change, if the change were wrong? I wanted to see "Test: xxx.html (No change behavior)", to confirm that the change is correct. Unfortunately, none. We can't check the appearance of search popup menus because DRT can't capture them.

Comment on attachment 130757 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130757&action=review >>> Source/WebCore/ChangeLog:11 >>> + No behavior change. >> >> Isn't there any existing test that _can_ be affected by this change, if the change were wrong? I wanted to see "Test: xxx.html (No change behavior)", to confirm that the change is correct. > > Unfortunately, none. We can't check the appearance of search popup menus because DRT can't capture them. We could grab the shadow tree using internals.shadowRoot() and dump its RenderTree using internals.elementRenderTreeAsText() You can make a reference DOM tree and compare the render-tree of the shadow tree and the referende DOM.

Comment on attachment 130757 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130757&action=review >>>> Source/WebCore/ChangeLog:11 >>>> + No behavior change. >>> >>> Isn't there any existing test that _can_ be affected by this change, if the change were wrong? I wanted to see "Test: xxx.html (No change behavior)", to confirm that the change is correct. >> >> Unfortunately, none. We can't check the appearance of search popup menus because DRT can't capture them. > > We could grab the shadow tree using internals.shadowRoot() and dump its RenderTree using internals.elementRenderTreeAsText() > You can make a reference DOM tree and compare the render-tree of the shadow tree and the referende DOM. It is not related to this patch. This patch doesn't change any renderer metrics or any DOM values. RenderTextControlSingleLine::clientInsetRight() and clientInsetLeft() are passed to platform-specific popup-menu implementation via ChromeClient::createSearchPopupMenu(). DRT doesn't have a way to get these values.

Comment on attachment 130757 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130757&action=review >>>>> Source/WebCore/ChangeLog:11 >>>>> + No behavior change. >>>> >>>> Isn't there any existing test that _can_ be affected by this change, if the change were wrong? I wanted to see "Test: xxx.html (No change behavior)", to confirm that the change is correct. >>> >>> Unfortunately, none. We can't check the appearance of search popup menus because DRT can't capture them. >> >> We could grab the shadow tree using internals.shadowRoot() and dump its RenderTree using internals.elementRenderTreeAsText() >> You can make a reference DOM tree and compare the render-tree of the shadow tree and the referende DOM. > > It is not related to this patch. > This patch doesn't change any renderer metrics or any DOM values. RenderTextControlSingleLine::clientInsetRight() and clientInsetLeft() are passed to platform-specific popup-menu implementation via ChromeClient::createSearchPopupMenu(). DRT doesn't have a way to get these values. Ah, got it. It would be nice if we have a mock for ChromeClient. But it's a different story...

Committed r110145: <http://trac.webkit.org/changeset/110145>

This is cool.