Summary: | [iOS] REGRESSION (r191849): There's no yellow bouncy highlight when using Find on Page | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||||||||||
Component: | New Bugs | Assignee: | Tim Horton <thorton> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | buildbot, commit-queue, kling, mcatanzaro, mkwst, rniwa, sam, simon.fraser | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Tim Horton
2015-12-14 00:52:45 PST
Created attachment 267284 [details]
Patch
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.
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. Comment on attachment 267284 [details]
Patch
Oh damn, sorry about that.
r=me!
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. Created attachment 267339 [details]
New patch with two tests
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? 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 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
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 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
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 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
> > 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! Somehow I forgot the rebaselines. Cmake build fix in http://trac.webkit.org/changeset/194118 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. |