Timeline should show when events preventDefault() was called on an event or not * Test <button id="b1">Default Not Prevent</button> <button id="b2">Default Prevented</button> <button onclick="sleep(); return false">Default Prevented Two</button> <script> function sleep() { let now = Date.now(); while ((Date.now() - now) < 100); } document.getElementById("b1").addEventListener("click", (event) => { sleep(); }); document.getElementById("b2").addEventListener("click", (event) => { sleep(); event.preventDefault(); }); </script> * Steps to reproduce: 1. Inspect test page 2. Start a timeline 3. Click each button 4. Stop recording => Should see which events had default prevented
<rdar://problem/34290931>
Created attachment 320617 [details] [PATCH] Proposed Fix - Lacking Tests
Created attachment 320618 [details] [IMAGE] Event Display
Noice. No spaces around em dash tho.
(In reply to Simon Fraser (smfr) from comment #4) > Noice. No spaces around em dash tho. I'm use the default styles for subtitles in tree elements. For example Resources has the same styles for "Name – domain".
Comment on attachment 320617 [details] [PATCH] Proposed Fix - Lacking Tests I'll shortly include a patch with tests.
Created attachment 320696 [details] [PATCH] Proposed Fix Now with tests!
Comment on attachment 320696 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=320696&action=review > LayoutTests/inspector/timeline/resources/timeline-event-utilities.js:21 > +function finishRecording(data = undefined) { I suppose at this point the = undefined is redundant.
Created attachment 320697 [details] [PATCH] Proposed Fix
Comment on attachment 320697 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=320697&action=review r=me > LayoutTests/inspector/timeline/resources/timeline-event-utilities.js:8 > + InspectorTest.log("Starting Capture..."); > + const newRecording = true; Style: I'd add a newline. > LayoutTests/inspector/timeline/resources/timeline-event-utilities.js:10 > + InspectorTest.evaluateInPage(expression); I would prefer you call this after adding the listener. > LayoutTests/inspector/timeline/timeline-event-EventDispatch.html:16 > + button.dispatchEvent(new MouseEvent("click", {bubbles: true, cancelable: true})); Can you just call `button.click();`? > LayoutTests/inspector/timeline/timeline-event-EventDispatch.html:21 > + button.dispatchEvent(new MouseEvent("click", {bubbles: true, cancelable: true})); Ditto. > LayoutTests/inspector/timeline/timeline-event-TimerFire.html:20 > + clearTimeout(setIntervalIdentifier); NIT: for consistency, I think `clearInterval(setIntervalIdentifier);` is preferred. > LayoutTests/inspector/timeline/timeline-event-TimerInstall.html:20 > + clearTimeout(setIntervalIdentifier); Ditto > LayoutTests/inspector/timeline/timeline-event-TimerRemove.html:19 > + clearTimeout(setIntervalIdentifier); Ditto > LayoutTests/inspector/timeline/timeline-event-TimerRemove.html:71 > + name: "SanityCheck", lol > Source/WebInspectorUI/UserInterface/Test/Test.js:42 > + InspectorBackend.registerDOMDispatcher(new WI.DOMObserver); Why did this move?
Comment on attachment 320697 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=320697&action=review >> LayoutTests/inspector/timeline/resources/timeline-event-utilities.js:10 >> + InspectorTest.evaluateInPage(expression); > > I would prefer you call this after adding the listener. I mostly find that approach harder to read. In practice these commands trigger async operations and are serial and so the events below cannot happen before. So I like to write it this way to logically group things. If we somehow rewrote tests to use a synchronous backend we'd encounter all kinds of issues. >> LayoutTests/inspector/timeline/timeline-event-TimerFire.html:20 >> + clearTimeout(setIntervalIdentifier); > > NIT: for consistency, I think `clearInterval(setIntervalIdentifier);` is preferred. Hah, I can't believe I totally forgot about `clearInterval`. Done! >> Source/WebInspectorUI/UserInterface/Test/Test.js:42 >> + InspectorBackend.registerDOMDispatcher(new WI.DOMObserver); > > Why did this move? To match the order of operations in Main.js. It isn't important, just consistency.
Created attachment 320724 [details] [PATCH] For Landing
Comment on attachment 320724 [details] [PATCH] For Landing Clearing flags on attachment: 320724 Committed r222015: <http://trac.webkit.org/changeset/222015>
(In reply to WebKit Commit Bot from comment #13) > Comment on attachment 320724 [details] > [PATCH] For Landing > > Clearing flags on attachment: 320724 > > Committed r222015: <http://trac.webkit.org/changeset/222015> Three of the tests added with this change are failing on macOS Debug WK1 bots: inspector/timeline/timeline-event-FireAnimationFrame.html inspector/timeline/timeline-event-TimerFire.html inspector/timeline/timeline-event-TimerRemove.html https://build.webkit.org/results/Apple%20Sierra%20Debug%20WK1%20(Tests)/r222016%20(3594)/results.html
Actually, it seems like these tests are flaky failures on WK2 as well: https://build.webkit.org/results/Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/r222016%20(3074)/results.html https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=inspector%2Ftimeline%2Ftimeline-event-
Reverted r222015 for reason: The LayoutTests added with this change are flaky. Committed r222030: <http://trac.webkit.org/changeset/222030>
Okay, I will avoid adding the Timer tests to this patch (they are kind of unrelated anyways) and investigate them separately. It doesn't appear the EventDispatch test was flakey like the timer ones.
Created attachment 320789 [details] [PATCH] For Landing
Comment on attachment 320789 [details] [PATCH] For Landing Clearing flags on attachment: 320789 Committed r222036: <http://trac.webkit.org/changeset/222036>
After the second patch the test inspector/timeline/timeline-event-EventDispatch.html is still very flaky. https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=inspector%2Ftimeline%2Ftimeline-event-EventDispatch.html https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=inspector%2Ftimeline%2Ftimeline-event-EventDispatch.html
Rolled out in: https://trac.webkit.org/changeset/222064/webkit It appears we have an existing bug in Timelines, that will have to be hunted down in order to test this. Likewise it may be more common in Debug, since running this test 50 times in release didn't exhibit any issue on my machines before I submitted it.
Created attachment 364999 [details] Patch
Comment on attachment 364999 [details] Patch Rejecting attachment 364999 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 364999, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Logging in as commit-queue@webkit.org... Fetching: https://bugs.webkit.org/attachment.cgi?id=364999&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=176824&ctype=xml&excludefield=attachmentdata Processing 1 patch from 1 bug. Processing patch 364999 from bug 176824. Fetching: https://bugs.webkit.org/attachment.cgi?id=364999 Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Timothy Hatcher']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Parsed 29 diffs from patch file(s). patching file Source/WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebInspectorUI/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebCore/dom/EventTarget.cpp patching file Source/WebCore/inspector/InspectorInstrumentation.cpp Hunk #1 succeeded at 413 with fuzz 1 (offset 2 lines). Hunk #2 succeeded at 431 (offset 2 lines). patching file Source/WebCore/inspector/InspectorInstrumentation.h Hunk #1 FAILED at 152. Hunk #2 succeeded at 347 with fuzz 2. Hunk #3 succeeded at 793 with fuzz 1. 1 out of 4 hunks FAILED -- saving rejects to file Source/WebCore/inspector/InspectorInstrumentation.h.rej patching file Source/WebCore/inspector/agents/InspectorTimelineAgent.cpp Hunk #1 succeeded at 306 (offset -3 lines). patching file Source/WebCore/inspector/agents/InspectorTimelineAgent.h Hunk #1 succeeded at 115 (offset -6 lines). patching file Source/WebCore/page/DOMWindow.cpp Hunk #1 succeeded at 2109 (offset 3 lines). patching file Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js Hunk #1 succeeded at 1178 (offset 8 lines). patching file Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js patching file Source/WebInspectorUI/UserInterface/Models/ScriptTimelineRecord.js patching file Source/WebInspectorUI/UserInterface/Views/ScriptDetailsTimelineView.js patching file Source/WebInspectorUI/UserInterface/Views/ScriptTimelineDataGridNode.js Hunk #1 succeeded at 86 (offset -14 lines). patching file LayoutTests/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file LayoutTests/inspector/timeline/resources/timeline-event-utilities.js patching file LayoutTests/inspector/timeline/timeline-event-CancelAnimationFrame-expected.txt patching file LayoutTests/inspector/timeline/timeline-event-CancelAnimationFrame.html patching file LayoutTests/inspector/timeline/timeline-event-EventDispatch-expected.txt patching file LayoutTests/inspector/timeline/timeline-event-EventDispatch.html patching file LayoutTests/inspector/timeline/timeline-event-FireAnimationFrame-expected.txt patching file LayoutTests/inspector/timeline/timeline-event-FireAnimationFrame.html patching file LayoutTests/inspector/timeline/timeline-event-RequestAnimationFrame-expected.txt patching file LayoutTests/inspector/timeline/timeline-event-RequestAnimationFrame.html patching file LayoutTests/inspector/timeline/timeline-event-TimerFire-expected.txt patching file LayoutTests/inspector/timeline/timeline-event-TimerFire.html patching file LayoutTests/inspector/timeline/timeline-event-TimerInstall-expected.txt patching file LayoutTests/inspector/timeline/timeline-event-TimerInstall.html patching file LayoutTests/inspector/timeline/timeline-event-TimerRemove-expected.txt patching file LayoutTests/inspector/timeline/timeline-event-TimerRemove.html Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Timothy Hatcher']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: https://webkit-queues.webkit.org/results/11583606
Created attachment 365390 [details] Patch
Nice!
Comment on attachment 365390 [details] Patch Clearing flags on attachment: 365390 Committed r243269: <https://trac.webkit.org/changeset/243269>
All reviewed patches have been landed. Closing bug.
Looks like all of the tests added in https://trac.webkit.org/changeset/243269/webkit are flaky: inspector/timeline/timeline-event-CancelAnimationFrame.html inspector/timeline/timeline-event-EventDispatch.html inspector/timeline/timeline-event-FireAnimationFrame.html inspector/timeline/timeline-event-RequestAnimationFrame.html inspector/timeline/timeline-event-TimerFire.html inspector/timeline/timeline-event-TimerInstall.html inspector/timeline/timeline-event-TimerRemove.html Combined History: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=inspector%2Ftimeline%2Ftimeline-event-CancelAnimationFrame.html%20inspector%2Ftimeline%2Ftimeline-event-EventDispatch.html%20inspector%2Ftimeline%2Ftimeline-event-FireAnimationFrame.html%20inspector%2Ftimeline%2Ftimeline-event-RequestAnimationFrame.html%20inspector%2Ftimeline%2Ftimeline-event-TimerFire.html%20inspector%2Ftimeline%2Ftimeline-event-TimerInstall.html%20inspector%2Ftimeline%2Ftimeline-event-TimerRemove.html
Looks like this also broke inspector/timeline/line-column.html again. History: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=inspector%2Ftimeline%2Fline-column.html
(In reply to Truitt Savell from comment #28) > Looks like all of the tests added in > https://trac.webkit.org/changeset/243269/webkit > > are flaky: > inspector/timeline/timeline-event-CancelAnimationFrame.html > inspector/timeline/timeline-event-EventDispatch.html > inspector/timeline/timeline-event-FireAnimationFrame.html > inspector/timeline/timeline-event-RequestAnimationFrame.html > inspector/timeline/timeline-event-TimerFire.html > inspector/timeline/timeline-event-TimerInstall.html > inspector/timeline/timeline-event-TimerRemove.html > > Combined History: > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > html#showAllRuns=true&tests=inspector%2Ftimeline%2Ftimeline-event- > CancelAnimationFrame.html%20inspector%2Ftimeline%2Ftimeline-event- > EventDispatch.html%20inspector%2Ftimeline%2Ftimeline-event- > FireAnimationFrame.html%20inspector%2Ftimeline%2Ftimeline-event- > RequestAnimationFrame.html%20inspector%2Ftimeline%2Ftimeline-event-TimerFire. > html%20inspector%2Ftimeline%2Ftimeline-event-TimerInstall. > html%20inspector%2Ftimeline%2Ftimeline-event-TimerRemove.html (In reply to Truitt Savell from comment #29) > Looks like this also broke inspector/timeline/line-column.html again. > > History: > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > html#showAllRuns=true&tests=inspector%2Ftimeline%2Fline-column.html (╯°□°)╯︵ ┻━┻
(In reply to Truitt Savell from comment #29) > Looks like this also broke inspector/timeline/line-column.html again. > > History: > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > html#showAllRuns=true&tests=inspector%2Ftimeline%2Fline-column.html Fixed in r243317 <https://trac.webkit.org/changeset/243317>. Still investigating the other failures.
(In reply to Truitt Savell from comment #28) > Looks like all of the tests added in > https://trac.webkit.org/changeset/243269/webkit > > are flaky: > inspector/timeline/timeline-event-CancelAnimationFrame.html > inspector/timeline/timeline-event-EventDispatch.html > inspector/timeline/timeline-event-FireAnimationFrame.html > inspector/timeline/timeline-event-RequestAnimationFrame.html > inspector/timeline/timeline-event-TimerFire.html > inspector/timeline/timeline-event-TimerInstall.html > inspector/timeline/timeline-event-TimerRemove.html > > Combined History: > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > html#showAllRuns=true&tests=inspector%2Ftimeline%2Ftimeline-event- > CancelAnimationFrame.html%20inspector%2Ftimeline%2Ftimeline-event- > EventDispatch.html%20inspector%2Ftimeline%2Ftimeline-event- > FireAnimationFrame.html%20inspector%2Ftimeline%2Ftimeline-event- > RequestAnimationFrame.html%20inspector%2Ftimeline%2Ftimeline-event-TimerFire. > html%20inspector%2Ftimeline%2Ftimeline-event-TimerInstall. > html%20inspector%2Ftimeline%2Ftimeline-event-TimerRemove.html Fixed in r243405 <http://trac.webkit.org/changeset/243405>.