| Summary: | Speedometer 2.0: jQuery test fails occasionally | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Shiyu Zhang <shiyu.zhang> | ||||||
| Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | addyo, buildbot, cdumez, commit-queue, lforschler, mathias, rniwa, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | PC | ||||||||
| OS: | Unspecified | ||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=176142 | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 172339 | ||||||||
| Attachments: |
|
||||||||
Thanks for highlighting this as a potential issue. Imo, Speedometer attempting to inject items before app.js is loaded is a bug (however this is the first time I've come across it). My only concern with the proposed solution is whether this pattern for readiness is consistent with how developers use jQuery. The fix looks good. Could you create a patch in accordance with https://webkit.org/contributing-code/ ? The easiest way to do this would be to run "./Tools/Scripts/wekbit-patch upload 176017 --commit-queue" at the top level directory, and then edit the change log with a description of your change. In the future, please cc me on any of Speedometer related bugs. I don't actively look for bugs so I'd totally miss a bug like this, and nobody else working on Speedometer 2.0 can legitimately review your patch since they're not a WebKit reviewer. Created attachment 319321 [details]
Patch
Thanks for reviewing the bug. I have created the patch, PTAL. I just noticed that BackboneJS suite may meet the same bug as well. The deleteButtons.length in resources/test.js for BackboneJS suite could be zero sometimes as I observed. This is even worse since unlike jQuery suite, BackboneJS suite will never crash even if deleteButtons.length is zero, which makes this bug difficult to be detected. Shall I file another bug to fix this issue? (In reply to Shiyu Zhang from comment #6) > I just noticed that BackboneJS suite may meet the same bug as well. The > deleteButtons.length in resources/test.js for BackboneJS suite could be zero > sometimes as I observed. This is even worse since unlike jQuery suite, > BackboneJS suite will never crash even if deleteButtons.length is zero, > which makes this bug difficult to be detected. Shall I file another bug to > fix this issue? Yes, please file a new bug and make it a blocker to 172339. Comment on attachment 319321 [details]
Patch
In the future, please also set cq flag to ? (to ask to be landed via commit queue).
Comment on attachment 319321 [details] Patch Clearing flags on attachment: 319321 Committed r221401: <http://trac.webkit.org/changeset/221401> All reviewed patches have been landed. Closing bug. |
Created attachment 319169 [details] A possible fix for jQuery test Speedometer 2.0 crashes at jQuery test with high possibility on my laptop (i5-5300U, win8, Chrome R60). Currently, jQuery test generates the DOM tree in its index.html and loads app.js, which binds events to the DOM nodes, at the end of the index.html. Speedometer framework starts "Adding items" action after the "new-todo" DOM node is ready. In jQuery test, there's a chance that Speedometer starts "Adding items" as soon as the "new-todo" node is ready while app.js has not been loaded completely and no event is binded to the DOM node yet. A possible fix patch is in the attachment.