- Revision
- 201840
- Author
- bb...@apple.com
- Date
- 2016-06-08 16:04:11 -0700 (Wed, 08 Jun 2016)
Log Message
Web Inspector: DOMTreeOutline selection areas should be created and updated lazily
https://bugs.webkit.org/show_bug.cgi?id=158513
<rdar://problem/26689646>
Reviewed by Timothy Hatcher.
Selection areas for DOMTreeElements are used for several things: drag markers,
element hover styles, element selection styles, and showing forced pseudo states
for an element. Fortunately it's easy to tell when any of these things is necessary.
Change DOMTreeOutline and DOMTreeElement so they don't create selection areas
unless they are needed for one of these tasks. This significantly reduces
forced layouts that are required to update the selection area height in case the
element has new attributes that cause the tag to become more or less wrapped.
* UserInterface/Views/DOMTreeElement.js:
(WebInspector.DOMTreeElement.prototype.set hovered):
Modernize this method a bit.
(WebInspector.DOMTreeElement.prototype.updateSelectionArea):
If a selection area is not necessary, don't create one.
If one exists and it's not needed, then remove it.
(WebInspector.DOMTreeElement.prototype.onattach):
Remove redundant calls to updateSelection(). This is already called in
updateTitle().
(WebInspector.DOMTreeElement.prototype.onselect):
Ask the DOMTreeOutline to update the selection rather than forcing the
element to do it. This is consistent with other updates to user selection.
(WebInspector.DOMTreeElement.prototype._insertInLastAttributePosition):
(WebInspector.DOMTreeElement.prototype._startEditingAsHTML.dispose):
(WebInspector.DOMTreeElement.prototype._startEditingAsHTML):
Use renamed method.
(WebInspector.DOMTreeElement.prototype.updateTitle):
Add a comment to explain why the selection area is nulled out here.
(WebInspector.DOMTreeElement.prototype.get pseudoClassesEnabled):
(WebInspector.DOMTreeElement.prototype._nodePseudoClassesDidChange):
Update the selection area in case one does not exist for this tree element.
The indicator for forced pseudo classes is a pseudo element of the selection area.
(WebInspector.DOMTreeElement.prototype.updateSelection): Renamed.
(WebInspector.DOMTreeElement.prototype.onexpand):
(WebInspector.DOMTreeElement.prototype.oncollapse):
Remove redundant calls to updateSelection(). This is already called in
updateTitle().
* UserInterface/Views/DOMTreeOutline.css:
(.tree-outline.dom):
(.tree-outline.dom li.hovered:not(.selected) .selection-area):
(.tree-outline.dom li .selection-area):
(.tree-outline.dom li.selected .selection-area):
(.tree-outline.dom li.elements-drag-over .selection-area):
(.tree-outline.dom:focus li.selected .selection-area):
(.tree-outline.dom li.pseudo-class-enabled > .selection-area::before):
(.tree-outline.dom:focus li.selected.pseudo-class-enabled > .selection-area::before):
(.tree-outline.dom li.hovered:not(.selected) .selection): Deleted.
(.tree-outline.dom li .selection): Deleted.
(.tree-outline.dom li.selected .selection): Deleted.
(.tree-outline.dom li.elements-drag-over .selection): Deleted.
(.tree-outline.dom:focus li.selected .selection): Deleted.
(.tree-outline.dom li.pseudo-class-enabled > .selection::before): Deleted.
(.tree-outline.dom:focus li.selected.pseudo-class-enabled > .selection::before): Deleted.
Rename the selector to be less ambiguous.
* UserInterface/Views/DOMTreeOutline.js:
(WebInspector.DOMTreeOutline.prototype.updateSelection): Simplify. The call
to update the selection area will bail out if there is nothing to be done.
(WebInspector.DOMTreeOutline.prototype.findTreeElement):
(WebInspector.DOMTreeOutline.prototype._onmousemove):
(WebInspector.DOMTreeOutline.prototype._onmouseout):
Clean up and use let and arrow functions.
(WebInspector.DOMTreeOutline.prototype._ondragover):
(WebInspector.DOMTreeOutline.prototype._clearDragOverTreeElementMarker):
Clear the dragging element before updating the selection area since it looks at
the dragging element to determine whether anything needs to be done.
* UserInterface/Views/FormattedValue.css:
(.formatted-node > .tree-outline.dom li.hovered:not(.selected) .selection-area):
(.formatted-node > .tree-outline.dom li.hovered:not(.selected) .selection): Deleted.
Rename the selector to be less ambiguous.
Modified Paths
Diff
Modified: trunk/Source/WebInspectorUI/ChangeLog (201839 => 201840)
--- trunk/Source/WebInspectorUI/ChangeLog 2016-06-08 23:02:39 UTC (rev 201839)
+++ trunk/Source/WebInspectorUI/ChangeLog 2016-06-08 23:04:11 UTC (rev 201840)
@@ -1,5 +1,94 @@
2016-06-08 Brian Burg <bb...@apple.com>
+ Web Inspector: DOMTreeOutline selection areas should be created and updated lazily
+ https://bugs.webkit.org/show_bug.cgi?id=158513
+ <rdar://problem/26689646>
+
+ Reviewed by Timothy Hatcher.
+
+ Selection areas for DOMTreeElements are used for several things: drag markers,
+ element hover styles, element selection styles, and showing forced pseudo states
+ for an element. Fortunately it's easy to tell when any of these things is necessary.
+
+ Change DOMTreeOutline and DOMTreeElement so they don't create selection areas
+ unless they are needed for one of these tasks. This significantly reduces
+ forced layouts that are required to update the selection area height in case the
+ element has new attributes that cause the tag to become more or less wrapped.
+
+ * UserInterface/Views/DOMTreeElement.js:
+ (WebInspector.DOMTreeElement.prototype.set hovered):
+ Modernize this method a bit.
+
+ (WebInspector.DOMTreeElement.prototype.updateSelectionArea):
+ If a selection area is not necessary, don't create one.
+ If one exists and it's not needed, then remove it.
+
+ (WebInspector.DOMTreeElement.prototype.onattach):
+ Remove redundant calls to updateSelection(). This is already called in
+ updateTitle().
+
+ (WebInspector.DOMTreeElement.prototype.onselect):
+ Ask the DOMTreeOutline to update the selection rather than forcing the
+ element to do it. This is consistent with other updates to user selection.
+
+ (WebInspector.DOMTreeElement.prototype._insertInLastAttributePosition):
+ (WebInspector.DOMTreeElement.prototype._startEditingAsHTML.dispose):
+ (WebInspector.DOMTreeElement.prototype._startEditingAsHTML):
+ Use renamed method.
+
+ (WebInspector.DOMTreeElement.prototype.updateTitle):
+ Add a comment to explain why the selection area is nulled out here.
+
+ (WebInspector.DOMTreeElement.prototype.get pseudoClassesEnabled):
+ (WebInspector.DOMTreeElement.prototype._nodePseudoClassesDidChange):
+ Update the selection area in case one does not exist for this tree element.
+ The indicator for forced pseudo classes is a pseudo element of the selection area.
+
+ (WebInspector.DOMTreeElement.prototype.updateSelection): Renamed.
+ (WebInspector.DOMTreeElement.prototype.onexpand):
+ (WebInspector.DOMTreeElement.prototype.oncollapse):
+ Remove redundant calls to updateSelection(). This is already called in
+ updateTitle().
+
+ * UserInterface/Views/DOMTreeOutline.css:
+ (.tree-outline.dom):
+ (.tree-outline.dom li.hovered:not(.selected) .selection-area):
+ (.tree-outline.dom li .selection-area):
+ (.tree-outline.dom li.selected .selection-area):
+ (.tree-outline.dom li.elements-drag-over .selection-area):
+ (.tree-outline.dom:focus li.selected .selection-area):
+ (.tree-outline.dom li.pseudo-class-enabled > .selection-area::before):
+ (.tree-outline.dom:focus li.selected.pseudo-class-enabled > .selection-area::before):
+ (.tree-outline.dom li.hovered:not(.selected) .selection): Deleted.
+ (.tree-outline.dom li .selection): Deleted.
+ (.tree-outline.dom li.selected .selection): Deleted.
+ (.tree-outline.dom li.elements-drag-over .selection): Deleted.
+ (.tree-outline.dom:focus li.selected .selection): Deleted.
+ (.tree-outline.dom li.pseudo-class-enabled > .selection::before): Deleted.
+ (.tree-outline.dom:focus li.selected.pseudo-class-enabled > .selection::before): Deleted.
+ Rename the selector to be less ambiguous.
+
+ * UserInterface/Views/DOMTreeOutline.js:
+ (WebInspector.DOMTreeOutline.prototype.updateSelection): Simplify. The call
+ to update the selection area will bail out if there is nothing to be done.
+
+ (WebInspector.DOMTreeOutline.prototype.findTreeElement):
+ (WebInspector.DOMTreeOutline.prototype._onmousemove):
+ (WebInspector.DOMTreeOutline.prototype._onmouseout):
+ Clean up and use let and arrow functions.
+
+ (WebInspector.DOMTreeOutline.prototype._ondragover):
+ (WebInspector.DOMTreeOutline.prototype._clearDragOverTreeElementMarker):
+ Clear the dragging element before updating the selection area since it looks at
+ the dragging element to determine whether anything needs to be done.
+
+ * UserInterface/Views/FormattedValue.css:
+ (.formatted-node > .tree-outline.dom li.hovered:not(.selected) .selection-area):
+ (.formatted-node > .tree-outline.dom li.hovered:not(.selected) .selection): Deleted.
+ Rename the selector to be less ambiguous.
+
+2016-06-08 Brian Burg <bb...@apple.com>
+
Uncaught Exception in TimelineDataGrid._updatePopoverForSelectedNode()
https://bugs.webkit.org/show_bug.cgi?id=158502
<rdar://problem/26687038>
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js (201839 => 201840)
--- trunk/Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js 2016-06-08 23:02:39 UTC (rev 201839)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js 2016-06-08 23:04:11 UTC (rev 201840)
@@ -144,20 +144,16 @@
return this._hovered;
}
- set hovered(x)
+ set hovered(value)
{
- if (this._hovered === x)
+ if (this._hovered === value)
return;
- this._hovered = x;
+ this._hovered = value;
if (this.listItemElement) {
- if (x) {
- this.updateSelection();
- this.listItemElement.classList.add("hovered");
- } else {
- this.listItemElement.classList.remove("hovered");
- }
+ this.listItemElement.classList.toggle("hovered", this._hovered);
+ this.updateSelectionArea();
}
}
@@ -262,27 +258,36 @@
WebInspector.RemoteObject.resolveNode(node, "", resolvedNode.bind(this));
}
- updateSelection()
+ updateSelectionArea()
{
- var listItemElement = this.listItemElement;
+ let listItemElement = this.listItemElement;
if (!listItemElement)
return;
- if (!this.selectionElement) {
- this.selectionElement = document.createElement("div");
- this.selectionElement.className = "selection selected";
- listItemElement.insertBefore(this.selectionElement, listItemElement.firstChild);
+ // If there's no reason to have a selection area, remove the DOM element.
+ let indicatesTreeOutlineState = this.treeOutline && (this.treeOutline.dragOverTreeElement === this || this.treeOutline.selectedTreeElement === this);
+ if (!this.hovered && !this.pseudoClassesEnabled && !indicatesTreeOutlineState) {
+ if (this._selectionElement) {
+ this._selectionElement.remove();
+ this._selectionElement = null;
+ }
+
+ return;
}
- this.selectionElement.style.height = listItemElement.offsetHeight + "px";
+ if (!this._selectionElement) {
+ this._selectionElement = document.createElement("div");
+ this._selectionElement.className = "selection-area";
+ listItemElement.insertBefore(this._selectionElement, listItemElement.firstChild);
+ }
+
+ this._selectionElement.style.height = listItemElement.offsetHeight + "px";
}
onattach()
{
- if (this._hovered) {
- this.updateSelection();
+ if (this.hovered)
this.listItemElement.classList.add("hovered");
- }
this.updateTitle();
@@ -484,7 +489,6 @@
return;
this.updateTitle();
- this.treeOutline.updateSelection();
}
oncollapse()
@@ -493,7 +497,6 @@
return;
this.updateTitle();
- this.treeOutline.updateSelection();
}
onreveal()
@@ -513,7 +516,7 @@
this.treeOutline.selectDOMNode(this.representedObject, selectedByUser);
if (selectedByUser)
WebInspector.domTreeManager.highlightDOMNode(this.representedObject.id);
- this.updateSelection();
+ this.treeOutline.updateSelection();
this.treeOutline.suppressRevealAndSelect = false;
}
@@ -588,7 +591,7 @@
tag.append("<" + nodeName, node, ">");
}
- this.updateSelection();
+ this.updateSelectionArea();
}
_startEditingTarget(eventTarget)
@@ -854,7 +857,7 @@
// Append editor.
this.listItemElement.appendChild(this._htmlEditElement);
- this.updateSelection();
+ this.updateSelectionArea();
function commit()
{
@@ -879,7 +882,7 @@
child = child.nextSibling;
}
- this.updateSelection();
+ this.updateSelectionArea();
}
var config = new WebInspector.EditingConfig(commit.bind(this), dispose.bind(this));
@@ -1073,8 +1076,10 @@
this._highlightResult = undefined;
}
- this.selectionElement = null;
- this.updateSelection();
+ // Setting this.title will implicitly remove all children. Clear the
+ // selection element so that we properly recreate it if necessary.
+ this._selectionElement = null;
+ this.updateSelectionArea();
this._highlightSearchResults();
}
@@ -1518,11 +1523,17 @@
}
}
+ get pseudoClassesEnabled()
+ {
+ return !!this.representedObject.enabledPseudoClasses.length;
+ }
+
_nodePseudoClassesDidChange(event)
{
if (this._elementCloseTag)
return;
+ this.updateSelectionArea();
this._listItemNode.classList.toggle("pseudo-class-enabled", !!this.representedObject.enabledPseudoClasses.length);
}
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.css (201839 => 201840)
--- trunk/Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.css 2016-06-08 23:02:39 UTC (rev 201839)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.css 2016-06-08 23:04:11 UTC (rev 201840)
@@ -34,16 +34,16 @@
list-style-type: none;
- /* Needed to make the negative z-index on .selection works. Otherwise the background-color from .syntax-highlighted hides the selection. */
+ /* Needed to make the negative z-index on .selection-area works. Otherwise the background-color from .syntax-highlighted hides the selection. */
background-color: transparent !important;
color: black;
}
-.tree-outline.dom li.hovered:not(.selected) .selection {
+.tree-outline.dom li.hovered:not(.selected) .selection-area {
background-color: hsla(209, 100%, 49%, 0.1);
}
-.tree-outline.dom li .selection {
+.tree-outline.dom li .selection-area {
position: absolute;
left: 0;
right: 0;
@@ -51,16 +51,16 @@
z-index: 10;
}
-.tree-outline.dom li.selected .selection {
+.tree-outline.dom li.selected .selection-area {
background-color: hsl(0, 0%, 83%);
}
-.tree-outline.dom li.elements-drag-over .selection {
+.tree-outline.dom li.elements-drag-over .selection-area {
margin-top: -2px;
border-top: 2px solid hsl(209, 100%, 49%);
}
-.tree-outline.dom:focus li.selected .selection {
+.tree-outline.dom:focus li.selected .selection-area {
background-color: hsl(209, 100%, 49%);
}
@@ -109,7 +109,7 @@
word-wrap: break-word;
}
-.tree-outline.dom li.pseudo-class-enabled > .selection::before {
+.tree-outline.dom li.pseudo-class-enabled > .selection-area::before {
display: inline-block;
position: absolute;
top: 4px;
@@ -129,7 +129,7 @@
color: white;
}
-.tree-outline.dom:focus li.selected.pseudo-class-enabled > .selection::before {
+.tree-outline.dom:focus li.selected.pseudo-class-enabled > .selection-area::before {
background-color: hsl(0, 100%, 100%);
}
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.js (201839 => 201840)
--- trunk/Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.js 2016-06-08 23:02:39 UTC (rev 201839)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.js 2016-06-08 23:04:11 UTC (rev 201840)
@@ -189,10 +189,12 @@
updateSelection()
{
- if (!this.selectedTreeElement)
- return;
- var element = this.treeOutline.selectedTreeElement;
- element.updateSelection();
+ // This will miss updating selection areas used for the hovered tree element and
+ // and those used to show forced pseudo class indicators, but this should be okay.
+ // The hovered element will update when user moves the mouse, and indicators don't need the
+ // selection area height to be accurate since they use ::before to place the indicator.
+ if (this.selectedTreeElement)
+ this.selectedTreeElement.updateSelectionArea();
}
_selectedNodeChanged()
@@ -202,17 +204,9 @@
findTreeElement(node)
{
- function isAncestorNode(ancestor, node)
- {
- return ancestor.isAncestor(node);
- }
-
- function parentNode(node)
- {
- return node.parentNode;
- }
-
- var treeElement = super.findTreeElement(node, isAncestorNode, parentNode);
+ let isAncestorNode = (ancestor, node) => ancestor.isAncestor(node);
+ let parentNode = (node) => node.parentNode;
+ let treeElement = super.findTreeElement(node, isAncestorNode, parentNode);
if (!treeElement && node.nodeType() === Node.TEXT_NODE) {
// The text node might have been inlined if it was short, so try to find the parent element.
treeElement = super.findTreeElement(node.parentNode, isAncestorNode, parentNode);
@@ -341,7 +335,7 @@
if (this._previousHoveredElement) {
this._previousHoveredElement.hovered = false;
- delete this._previousHoveredElement;
+ this._previousHoveredElement = null;
}
if (element) {
@@ -364,7 +358,7 @@
if (this._previousHoveredElement) {
this._previousHoveredElement.hovered = false;
- delete this._previousHoveredElement;
+ this._previousHoveredElement = null;
}
WebInspector.domTreeManager.hideDOMNodeHighlight();
@@ -407,9 +401,10 @@
node = node.parentNode;
}
- treeElement.updateSelection();
+ this.dragOverTreeElement = treeElement;
treeElement.listItemElement.classList.add("elements-drag-over");
- this._dragOverTreeElement = treeElement;
+ treeElement.updateSelectionArea();
+
event.preventDefault();
event.dataTransfer.dropEffect = "move";
return false;
@@ -481,10 +476,12 @@
_clearDragOverTreeElementMarker()
{
- if (this._dragOverTreeElement) {
- this._dragOverTreeElement.updateSelection();
- this._dragOverTreeElement.listItemElement.classList.remove("elements-drag-over");
- delete this._dragOverTreeElement;
+ if (this.dragOverTreeElement) {
+ let element = this.dragOverTreeElement;
+ this.dragOverTreeElement = null;
+
+ element.listItemElement.classList.remove("elements-drag-over");
+ element.updateSelectionArea();
}
}
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/FormattedValue.css (201839 => 201840)
--- trunk/Source/WebInspectorUI/UserInterface/Views/FormattedValue.css 2016-06-08 23:02:39 UTC (rev 201839)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/FormattedValue.css 2016-06-08 23:04:11 UTC (rev 201840)
@@ -76,7 +76,7 @@
-webkit-user-select: none !important;
}
-.formatted-node > .tree-outline.dom li.hovered:not(.selected) .selection {
+.formatted-node > .tree-outline.dom li.hovered:not(.selected) .selection-area {
display: block;
left: -1px;
right: -2px;
