Summary: | Revert r127457 and following fixes due to several hit-testing regressions | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Julien Chaffraix <jchaffraix> | ||||||
Component: | UI Events | Assignee: | Allan Sandfeld Jensen <allan.jensen> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | allan.jensen, ap, cdumez, eric, jamesr, jberlin, tonikitoo, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Bug Depends on: | 96929 | ||||||||
Bug Blocks: | 95204 | ||||||||
Attachments: |
|
Description
Julien Chaffraix
2012-09-14 15:19:31 PDT
Created attachment 164236 [details]
Proposed revert.
Committed r128677: <http://trac.webkit.org/changeset/128677> Technically there are also two lines of code the patch moving Document::updateActiveHoverState that should be reverted. It is the two lines that find the inner node of the current document. They don't do anything wrong, but are unnecessary now that innerNode is always in the current document again. Created attachment 164355 [details]
Follow-up Patch
Revert two additional lines of code. Keeping these would have been confusing, and they would have been essentially dead.
Comment on attachment 164236 [details] Proposed revert. Rejecting attachment 164236 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: frame.html, which does not exist! Applying it anyway. patching file LayoutTests/touchadjustment/resources/inner-navigation-frame.html Hunk #1 FAILED at 1. 1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/touchadjustment/resources/inner-navigation-frame.html.rej patching file ChangeLog Hunk #1 succeeded at 1 with fuzz 3. Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Antonio Go..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://queues.webkit.org/results/13868381 Comment on attachment 164236 [details]
Proposed revert.
clearing flags for landed patch
Comment on attachment 164355 [details] Follow-up Patch Clearing flags on attachment: 164355 Committed r128759: <http://trac.webkit.org/changeset/128759> All reviewed patches have been landed. Closing bug. Comment on attachment 164355 [details] Follow-up Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164355&action=review > Source/WebCore/dom/Document.cpp:6157 > + ASSERT(innerNodeInDocument->document() == this); We are hitting this assertion in several tests now (at least on EFL port) : http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug/r128762%20(5986)/results.html (In reply to comment #9) > (From update of attachment 164355 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=164355&action=review > > > Source/WebCore/dom/Document.cpp:6157 > > + ASSERT(innerNodeInDocument->document() == this); > > We are hitting this assertion in several tests now (at least on EFL port) : > http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug/r128762%20(5986)/results.html It seems to hit the assertion on Mac as well: http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20WK1%20%28Tests%29/builds/924 Exited early after 20 crashes. Re-opened since this is blocked by 96929 Comment on attachment 164355 [details] Follow-up Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164355&action=review >>> Source/WebCore/dom/Document.cpp:6157 >>> + ASSERT(innerNodeInDocument->document() == this); >> >> We are hitting this assertion in several tests now (at least on EFL port) : >> http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug/r128762%20(5986)/results.html > > It seems to hit the assertion on Mac as well: > http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20WK1%20%28Tests%29/builds/924 > > Exited early after 20 crashes. The assertion forgets to check for innerNode being null. ASSERT(!innerNodeInDocument || innerNodeInDocument->document() == this); Last patch landed as http://trac.webkit.org/changeset/128869 |