RESOLVED FIXED149742
Regression: Web Inspector: Sometimes in Elements panel two elements showed as selected at the same time
https://bugs.webkit.org/show_bug.cgi?id=149742
Summary Regression: Web Inspector: Sometimes in Elements panel two elements showed as...
Nikita Vasilyev
Reported 2015-10-02 00:38:51 PDT
Created attachment 262321 [details] [Animated GIF] Bug See the attached GIF. So far I was unable to find exact steps to reproduce the problem.
Attachments
[Animated GIF] Bug (283.61 KB, image/gif)
2015-10-02 00:38 PDT, Nikita Vasilyev
no flags
[PATCH] Proposed Fix (2.99 KB, patch)
2016-02-08 22:08 PST, Joseph Pecoraro
no flags
Radar WebKit Bug Importer
Comment 1 2015-10-02 00:39:05 PDT
Nikita Vasilyev
Comment 2 2015-10-02 00:40:34 PDT
It's started happening for me a couple of months ago. Possibly a regression.
Joseph Pecoraro
Comment 3 2015-10-02 10:47:46 PDT
Were there exceptions in the inspector?
Nikita Vasilyev
Comment 4 2015-10-04 21:09:19 PDT
(In reply to comment #3) > Were there exceptions in the inspector? No.
Nikita Vasilyev
Comment 5 2015-10-08 00:06:38 PDT
I recently observed this exception: Assertion Failed markAsNeedsRefresh — StyleDetailsPanel.js:85 visibilityDidChange — CSSStyleDetailsSidebarPanel.js:148 removeSidebarPanel — Sidebar.js:98 showDetailsSidebarPanels — ContentBrowserTabContentView.js:168 dispatch — Object.js:151 dispatchEventToListeners — Object.js:158 _dispatchCurrentRepresentedObjectsDidChangeEvent — ContentBrowser.js:437
Timothy Hatcher
Comment 6 2015-10-08 07:57:06 PDT
That is an assert, not an exception. So it wouldn't cause random breakage. We should file a separate bug for that.
Joseph Pecoraro
Comment 7 2016-02-08 21:54:45 PST
Joseph Pecoraro
Comment 8 2016-02-08 21:56:49 PST
* SUMMARY Regression produced by r187496. * TEST <body> <div> <p>Test</p> <p>Test</p> <p>Test</p> <p>Test</p> <button id="start">Start</button> <button id="stop">Stop</button> </div> <script> setInterval(function() { var elem = document.body.appendChild(document.createElement("div")); elem.style.cssText = "width: 100px; height: 100px; background: blue"; setTimeout(function() { document.body.removeChild(elem); }, 0); }, 500); </script> * STEPS TO REPRODUCE 1. Inspect a <p> on the test page 2. Click the other <p> tags => multiple selections left over * NOTES - The TreeOutline loses the selectedTreeElement, but the inner selected TreeElement still sees itself as selected...
Joseph Pecoraro
Comment 9 2016-02-08 22:08:37 PST
Created attachment 270913 [details] [PATCH] Proposed Fix This fixes the issue, and keyboard navigation within the DOM Tree gets fixed as well (previously when this was happened the tree would lose focus). However I'm marking as cq- because I still fail to see how this was caused by r187496, which means there is something I don't yet fully understand here, so I want to investigate this a bit more.
Joseph Pecoraro
Comment 10 2016-02-09 21:15:25 PST
> However I'm marking as cq- because I still fail to see how this was caused > by r187496, which means there is something I don't yet fully understand > here, so I want to investigate this a bit more. I now understand! The same optimization that I'm adding to moveChild(...) effectively happened before. While iterating over the child list the old code would skip over tree elements in the correct place: > while (child) { > var currentTreeElement = treeElement.children[treeChildIndex]; > if (!currentTreeElement || currentTreeElement.representedObject !== child) { > ... > } > ... > } Basically we now do that first check in moveChild(...). The old code could presumably have a bug that this addresses with the correct selecting. Either way, I'm now confident that this can land.
WebKit Commit Bot
Comment 11 2016-02-09 22:02:54 PST
Comment on attachment 270913 [details] [PATCH] Proposed Fix Clearing flags on attachment: 270913 Committed r196362: <http://trac.webkit.org/changeset/196362>
WebKit Commit Bot
Comment 12 2016-02-09 22:02:57 PST
All reviewed patches have been landed. Closing bug.
Timothy Hatcher
Comment 13 2016-02-24 11:24:29 PST
*** Bug 154645 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.