Bug 152241 – [iOS] REGRESSION (r191849): There's no yellow bouncy highlight when using Find on Page

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tim Horton]

[iOS] REGRESSION (r191849): There's no yellow bouncy highlight when using Find on Page

[Tim Horton]

Created attachment 267284 [details]
Patch

[WebKit Commit Bot]

Attachment 267284 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tim Horton]

Going to figure out how to make a test, because this is fairly fundamental functionality that was broken for months with no tests catching it.

[Andreas Kling]

Comment on attachment 267284 [details]
Patch

Oh damn, sorry about that.
r=me!

[Darin Adler]

Comment on attachment 267284 [details]
Patch

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

> Source/WebCore/page/PageOverlay.cpp:78
> +        return IntRect(IntPoint(), m_overrideFrame.size());

I like to write it like this:

    return { { }, m_overrideFrame.size() };

Maybe less clear, but I like how it’s more terse and the type is implicit and so it’s guaranteed no extra type conversion happens.

[Tim Horton]

Created attachment 267339 [details]
New patch with two tests

[Simon Fraser (smfr)]

Comment on attachment 267339 [details]
New patch with two tests

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

> Source/WebCore/ChangeLog:15
> +        if we frame is manually overriden.

if we frame?

> Source/WebCore/ChangeLog:47
> +        * testing/MockPageOverlay.cpp: Added.
> +        (WebCore::MockPageOverlay::create):
> +        (WebCore::MockPageOverlay::MockPageOverlay):
> +        (WebCore::MockPageOverlay::setFrame):
> +        * testing/MockPageOverlay.h: Added.
> +        (WebCore::MockPageOverlay::overlay):
> +        * testing/MockPageOverlay.idl: Added.
> +        * testing/MockPageOverlayClient.cpp:
> +        (WebCore::MockPageOverlayClient::installOverlay):
> +        (WebCore::MockPageOverlayClient::uninstallAllOverlays):
> +        (WebCore::MockPageOverlayClient::pageOverlayDestroyed):
> +        (WebCore::MockPageOverlayClient::drawRect):
> +        (WebCore::MockPageOverlayClient::mouseEvent):
> +        * testing/MockPageOverlayClient.h:
> +        Make internals.installMockPageOverlay return a MockPageOverlay object so
> +        tests can manipulate their overlay. For now, expose setFrame.
> +        Also, log when MockPageOverlayClient gets asked to paint or receives a mouse event,
> +        which will show up in test output. Slightly unconventional, but very convenient.

I'm close to saying that you've gone too far here. Does all this code ship?

> Source/WebCore/page/PageOverlay.cpp:190
> +    mousePositionInOverlayCoordinates.moveBy(-frame().location());

Doesn't the overlay have a "contentsToOverlay" or something that should take care of this?

[Build Bot]

Comment on attachment 267339 [details]
New patch with two tests

Attachment 267339 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/560885

New failing tests:
pageoverlay/overlay-large-document-scrolled.html
pageoverlay/overlay-installation.html
pageoverlay/overlay-small-frame-mouse-events.html
pageoverlay/overlay-large-document.html

[Build Bot]

Created attachment 267345 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 267339 [details]
New patch with two tests

Attachment 267339 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/560879

New failing tests:
pageoverlay/overlay-large-document.html
pageoverlay/overlay-installation.html
pageoverlay/overlay-large-document-scrolled.html
pageoverlay/overlay-small-frame-mouse-events.html

[Build Bot]

Created attachment 267346 [details]
Archive of layout-test-results from ews112 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 267339 [details]
New patch with two tests

Attachment 267339 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/560887

New failing tests:
pageoverlay/overlay-large-document.html
pageoverlay/overlay-installation.html
pageoverlay/overlay-large-document-scrolled.html
pageoverlay/overlay-small-frame-mouse-events.html

[Build Bot]

Created attachment 267347 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Tim Horton]

> > Source/WebCore/ChangeLog:47
> > +        * testing/MockPageOverlay.cpp: Added.
> > +        (WebCore::MockPageOverlay::create):
> > +        (WebCore::MockPageOverlay::MockPageOverlay):
> > +        (WebCore::MockPageOverlay::setFrame):
> > +        * testing/MockPageOverlay.h: Added.
> > +        (WebCore::MockPageOverlay::overlay):
> > +        * testing/MockPageOverlay.idl: Added.
> > +        * testing/MockPageOverlayClient.cpp:
> > +        (WebCore::MockPageOverlayClient::installOverlay):
> > +        (WebCore::MockPageOverlayClient::uninstallAllOverlays):
> > +        (WebCore::MockPageOverlayClient::pageOverlayDestroyed):
> > +        (WebCore::MockPageOverlayClient::drawRect):
> > +        (WebCore::MockPageOverlayClient::mouseEvent):
> > +        * testing/MockPageOverlayClient.h:
> > +        Make internals.installMockPageOverlay return a MockPageOverlay object so
> > +        tests can manipulate their overlay. For now, expose setFrame.
> > +        Also, log when MockPageOverlayClient gets asked to paint or receives a mouse event,
> > +        which will show up in test output. Slightly unconventional, but very convenient.
> 
> I'm close to saying that you've gone too far here.

I'd say we're pretty far past too far.

> Does all this code ship?

No, it's built into WebCoreTestSupport!

[Tim Horton]

Somehow I forgot the rebaselines.

[Tim Horton]

http://trac.webkit.org/changeset/194117

[Tim Horton]

Cmake build fix in http://trac.webkit.org/changeset/194118

[Michael Catanzaro]

Loren discovered the following tests have been failing for GTK since r194117:

pageoverlay/overlay-installation.html [ Failure ]
pageoverlay/overlay-large-document-scrolled.html [ Failure ]
pageoverlay/overlay-large-document.html [ Failure ]
pageoverlay/overlay-small-frame-mouse-events.html [ Failure ]
pageoverlay/overlay-small-frame-paints.html [ Failure ]

The test are not showing the 'MockPageOverlayClient::drawRect dirtyRect()' logs introduced on the expectations.

He filed bug #152908.