- Revision
- 174510
- Author
- [email protected]
- Date
- 2014-10-09 10:34:06 -0700 (Thu, 09 Oct 2014)
Log Message
Web Inspector: DOM Storage Editing is broken in many ways, frustrating
https://bugs.webkit.org/show_bug.cgi?id=137547
Patch by Joseph Pecoraro <[email protected]> on 2014-10-09
Reviewed by Timothy Hatcher.
* UserInterface/Models/DOMStorageObject.js:
(WebInspector.DOMStorageObject.prototype.getEntries): Deleted.
When getting entires, populate the model object with these keys/values
so we can accurately detect duplicates in editing.
* UserInterface/Views/DataGrid.js:
(WebInspector.DataGrid.prototype.determineNextCell):
(WebInspector.DataGrid.prototype.moveToNextCell):
When the users uses "Enter" to commit, stop editing.
(WebInspector.DataGrid.prototype._editingCommitted):
Fix title property accessor. This is not a map. This fixes an exception
when showing context menus on editable data grid rows.
* UserInterface/Views/DOMStorageContentView.js:
(WebInspector.DOMStorageContentView.prototype.cleanup):
(WebInspector.DOMStorageContentView.prototype.restoreOriginalValues):
(WebInspector.DOMStorageContentView.prototype._editingCallback):
Completely rewrite editing here. As soon as an edit is made, enter
an uncommitted state with the original key/value. When a commit is
allowed, update as appropriate based on the original values.
Modified Paths
Diff
Modified: trunk/Source/WebInspectorUI/ChangeLog (174509 => 174510)
--- trunk/Source/WebInspectorUI/ChangeLog 2014-10-09 17:01:07 UTC (rev 174509)
+++ trunk/Source/WebInspectorUI/ChangeLog 2014-10-09 17:34:06 UTC (rev 174510)
@@ -1,3 +1,32 @@
+2014-10-09 Joseph Pecoraro <[email protected]>
+
+ Web Inspector: DOM Storage Editing is broken in many ways, frustrating
+ https://bugs.webkit.org/show_bug.cgi?id=137547
+
+ Reviewed by Timothy Hatcher.
+
+ * UserInterface/Models/DOMStorageObject.js:
+ (WebInspector.DOMStorageObject.prototype.getEntries): Deleted.
+ When getting entires, populate the model object with these keys/values
+ so we can accurately detect duplicates in editing.
+
+ * UserInterface/Views/DataGrid.js:
+ (WebInspector.DataGrid.prototype.determineNextCell):
+ (WebInspector.DataGrid.prototype.moveToNextCell):
+ When the users uses "Enter" to commit, stop editing.
+
+ (WebInspector.DataGrid.prototype._editingCommitted):
+ Fix title property accessor. This is not a map. This fixes an exception
+ when showing context menus on editable data grid rows.
+
+ * UserInterface/Views/DOMStorageContentView.js:
+ (WebInspector.DOMStorageContentView.prototype.cleanup):
+ (WebInspector.DOMStorageContentView.prototype.restoreOriginalValues):
+ (WebInspector.DOMStorageContentView.prototype._editingCallback):
+ Completely rewrite editing here. As soon as an edit is made, enter
+ an uncommitted state with the original key/value. When a commit is
+ allowed, update as appropriate based on the original values.
+
2014-10-08 Joseph Pecoraro <[email protected]>
Web Inspector: CSS Pretty Printing fails to put space between value functions around ending parenthesis
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/DOMStorageObject.js (174509 => 174510)
--- trunk/Source/WebInspectorUI/UserInterface/Models/DOMStorageObject.js 2014-10-09 17:01:07 UTC (rev 174509)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/DOMStorageObject.js 2014-10-09 17:34:06 UTC (rev 174510)
@@ -74,11 +74,25 @@
getEntries: function(callback)
{
+ function innerCallback(error, entries)
+ {
+ if (error)
+ return;
+
+ for (var entry of entries) {
+ if (!entry[0] || !entry[1])
+ continue;
+ this._entries.set(entry[0], entry[1]);
+ }
+
+ callback(error, entries);
+ }
+
// COMPATIBILITY (iOS 6): The getDOMStorageItems function was later renamed to getDOMStorageItems.
if (DOMStorageAgent.getDOMStorageEntries)
- DOMStorageAgent.getDOMStorageEntries(this._id, callback);
+ DOMStorageAgent.getDOMStorageEntries(this._id, innerCallback.bind(this));
else
- DOMStorageAgent.getDOMStorageItems(this._id, callback);
+ DOMStorageAgent.getDOMStorageItems(this._id, innerCallback.bind(this));
},
removeItem: function(key)
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/DOMStorageContentView.js (174509 => 174510)
--- trunk/Source/WebInspectorUI/UserInterface/Views/DOMStorageContentView.js 2014-10-09 17:01:07 UTC (rev 174509)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/DOMStorageContentView.js 2014-10-09 17:34:06 UTC (rev 174510)
@@ -185,58 +185,95 @@
_editingCallback: function(editingNode, columnIdentifier, oldText, newText, moveDirection)
{
- var key = editingNode.data["key"].trim();
- var value = editingNode.data["value"].trim();
- var previousValue = oldText.trim();
- var enteredValue = newText.trim();
- var columnIndex = this._dataGrid.orderedColumns.indexOf(columnIdentifier);
- var mayMoveToNextRow = moveDirection === "forward" && columnIndex === this._dataGrid.orderedColumns.length - 1;
- var mayMoveToPreviousRow = moveDirection === "backward" && columnIndex === 0;
- var willMoveRow = mayMoveToNextRow || mayMoveToPreviousRow;
- var shouldCommitRow = willMoveRow && key.length && value.length;
+ var key = editingNode.data["key"].trim().removeWordBreakCharacters();
+ var value = editingNode.data["value"].trim().removeWordBreakCharacters();
+ var previousValue = oldText.trim().removeWordBreakCharacters();
+ var enteredValue = newText.trim().removeWordBreakCharacters();
+ var hasUncommittedEdits = editingNode.__hasUncommittedEdits;
+ var hasChange = previousValue !== enteredValue;
+ var isEditingKey = columnIdentifier === "key";
+ var isEditingValue = !isEditingKey;
+ var domStorage = this.representedObject;
- // Remove the row if its values are newly cleared, and it's not a placeholder.
- if (!key.length && !value.length && willMoveRow) {
- if (previousValue.length && !editingNode.isPlaceholderNode)
- this._dataGrid.removeChild(editingNode);
+ // Nothing changed, just bail.
+ if (!hasChange && !hasUncommittedEdits)
return;
+
+ // Something changed, save the original key/value and enter uncommitted state.
+ if (hasChange && !editingNode.__hasUncommittedEdits) {
+ editingNode.__hasUncommittedEdits = true;
+ editingNode.__originalKey = isEditingKey ? previousValue : key;
+ editingNode.__originalValue = isEditingValue ? previousValue : value;
}
- // If the key field was deleted, restore it when committing the row.
- if (key === enteredValue && !key.length) {
- if (willMoveRow && !editingNode.isPlaceholderNode) {
- editingNode.data.key = previousValue;
- editingNode.refresh();
- } else
- editingNode.element.classList.add(WebInspector.DOMStorageContentView.MissingKeyStyleClassName);
- } else if (key.length) {
+ function cleanup()
+ {
editingNode.element.classList.remove(WebInspector.DOMStorageContentView.MissingKeyStyleClassName);
- editingNode.__previousKeyValue = previousValue;
+ editingNode.element.classList.remove(WebInspector.DOMStorageContentView.MissingValueStyleClassName);
+ editingNode.element.classList.remove(WebInspector.DOMStorageContentView.DuplicateKeyStyleClassName);
+ delete editingNode.__hasUncommittedEdits;
+ delete editingNode.__originalKey;
+ delete editingNode.__originalValue;
}
- if (value === enteredValue && !value.length)
- editingNode.element.classList.add(WebInspector.DOMStorageContentView.MissingValueStyleClassName);
- else
- editingNode.element.classList.remove(WebInspector.DOMStorageContentView.MissingValueStyleClassName);
+ function restoreOriginalValues()
+ {
+ editingNode.data.key = editingNode.__originalKey;
+ editingNode.data.value = editingNode.__originalValue;
+ editingNode.refresh();
+ cleanup();
+ }
- if (editingNode.isPlaceholderNode && previousValue !== enteredValue)
- this._dataGrid.addPlaceholderNode();
+ // If the key/value field was cleared, add "missing" style.
+ if (isEditingKey) {
+ if (key.length)
+ editingNode.element.classList.remove(WebInspector.DOMStorageContentView.MissingKeyStyleClassName);
+ else
+ editingNode.element.classList.add(WebInspector.DOMStorageContentView.MissingKeyStyleClassName);
+ } else if (isEditingValue) {
+ if (value.length)
+ editingNode.element.classList.remove(WebInspector.DOMStorageContentView.MissingValueStyleClassName);
+ else
+ editingNode.element.classList.add(WebInspector.DOMStorageContentView.MissingValueStyleClassName);
+ }
- if (!shouldCommitRow)
- return; // One of the inputs is missing, or we aren't moving between rows.
+ // Check for key duplicates. If this is a new row, or an existing row that changed key.
+ var keyChanged = key !== editingNode.__originalKey;
+ if (keyChanged) {
+ if (domStorage.entries.has(key))
+ editingNode.element.classList.add(WebInspector.DOMStorageContentView.DuplicateKeyStyleClassName);
+ else
+ editingNode.element.classList.remove(WebInspector.DOMStorageContentView.DuplicateKeyStyleClassName);
+ }
- var domStorage = this.representedObject;
- if (domStorage.entries.has(key)) {
- editingNode.element.classList.add(WebInspector.DOMStorageContentView.DuplicateKeyStyleClassName);
+ // See if we are done editing this row or not.
+ var columnIndex = this._dataGrid.orderedColumns.indexOf(columnIdentifier);
+ var mayMoveToNextRow = moveDirection === "forward" && columnIndex === this._dataGrid.orderedColumns.length - 1;
+ var mayMoveToPreviousRow = moveDirection === "backward" && columnIndex === 0;
+ var doneEditing = mayMoveToNextRow || mayMoveToPreviousRow || !moveDirection;
+
+ // Expecting more edits on this row.
+ if (!doneEditing)
return;
+
+ // Key and value were cleared, remove the row.
+ if (!key.length && !value.length && !editingNode.isPlaceholderNode) {
+ this._dataGrid.removeChild(editingNode);
+ domStorage.removeItem(editingNode.__originalKey);
+ return;
}
- editingNode.element.classList.remove(WebInspector.DOMStorageContentView.DuplicateKeySyleClassName);
+ // Done editing but leaving the row in an invalid state. Leave in uncommitted state.
+ var isDuplicate = editingNode.element.classList.contains(WebInspector.DOMStorageContentView.DuplicateKeyStyleClassName);
+ if (!key.length || !value.length || isDuplicate)
+ return;
- if (editingNode.__previousKeyValue !== key)
- domStorage.removeItem(editingNode.__previousKeyValue);
-
+ // Commit.
+ if (keyChanged && !editingNode.isPlaceholderNode)
+ domStorage.removeItem(editingNode.__originalKey);
+ if (editingNode.isPlaceholderNode)
+ this._dataGrid.addPlaceholderNode();
+ cleanup();
domStorage.setItem(key, value);
- // The table will be re-sorted when the backend fires the itemUpdated event.
}
};
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/DataGrid.js (174509 => 174510)
--- trunk/Source/WebInspectorUI/UserInterface/Views/DataGrid.js 2014-10-09 17:01:07 UTC (rev 174509)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/DataGrid.js 2014-10-09 17:34:06 UTC (rev 174510)
@@ -316,8 +316,8 @@
return {shouldSort: true, editingNode: previousDataGridNode || currentEditingNode, columnIndex: this.orderedColumns.length - 1};
}
- // If we are not moving in any direction, then sort but don't move.
- return {shouldSort: true, editingNode: currentEditingNode, columnIndex: columnIndex};
+ // If we are not moving in any direction, then sort and stop.
+ return {shouldSort: true};
}
function moveToNextCell(valueDidChange) {
@@ -326,7 +326,8 @@
this._sortAfterEditingCallback();
delete this._sortAfterEditingCallback;
}
- this._startEditingNodeAtColumnIndex(moveCommand.editingNode, moveCommand.columnIndex);
+ if (moveCommand.editingNode)
+ this._startEditingNodeAtColumnIndex(moveCommand.editingNode, moveCommand.columnIndex);
}
this._editingCancelled(element);
@@ -1145,7 +1146,7 @@
else {
var element = event.target.enclosingNodeOrSelfWithNodeName("td");
var columnIdentifier = element.__columnIdentifier;
- var columnTitle = this.dataGrid.columns.get(columnIdentifier).get("title");
+ var columnTitle = this.dataGrid.columns.get(columnIdentifier)["title"];
contextMenu.appendItem(WebInspector.UIString("Edit “%s”").format(columnTitle), this._startEditing.bind(this, event.target));
}
}
