| Summary: | Web Inspector: Styles: Loses focus when editing a property while page is being loaded | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Nikita Vasilyev <nvasilyev> | ||||||||||||||||||
| Component: | Web Inspector | Assignee: | Nikita Vasilyev <nvasilyev> | ||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||
| Severity: | Normal | CC: | commit-queue, ews-watchlist, inspector-bugzilla-changes, mattbaker, timothy, webkit-bug-importer | ||||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||||||
| Hardware: | All | ||||||||||||||||||||
| OS: | All | ||||||||||||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=183755 | ||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||
|
Description
Nikita Vasilyev
2018-02-08 13:20:05 PST
It looks like sections (SpreadsheetCSSStyleDeclarationSection) are getting removed and added again while being edited. Something during page load causes a full re-layout.
SpreadsheetRulesStyleDetailsPanel.js:
refresh(significantChange)
{
// We only need to do a rebuild on significant changes. Other changes are handled
// by the sections and text editors themselves.
if (!significantChange) {
super.refresh(significantChange);
return;
}
this.removeAllSubviews();
Perhaps SpreadsheetRulesStyleDetailsPanel should be smarter about re-layouts.
P.S. Things that would help to debug this:
— Better filtering in Timelines. Timelines are spammed by codemirror.js. I want to exclude all stacktraces that have calls from codemirror.js
— View.js stacktraces. I want to a stacktrace of where the layout was scheduled.
This bug has other unpleasant consequences. After losing the focus, CSSStyleDeclaration stays locked. Clicking on the white space does not add a blank property. When focused on a CSS property name or value, CSSStyleDeclaration gets locked. This was added in Bug 179461 - Web Inspector: Styles Redesign: data corruption when updating values quickly. CSSStyleDeclaration gets unlocked on "blur" event. Blur event does NOT fire on a focused element when it detaches from DOM. (Ryosuke said it was changed in WebKit about a year ago.) SpreadsheetCSSStyleDeclarationEditor.prototype.layout rebuilds every CSS property view, even when the properties are unchanged. Not only it's a lot of unnecessary work, rebuilding a focused CSS property makes it lose the focus without firing "blur" event. Created attachment 335657 [details]
Patch
Created attachment 335658 [details]
[Animated GIF] With patch applied
theverge.com was unreliable as a reduction page, so I made a small page myself: http://nv.github.io/webkit-inspector-bugs/styles-redesign/tests/style-added-removed-via-js.html Steps: 1. Open Web Inspector 2. Select <body> in Elements tab 3. Navigate to http://nv.github.io/webkit-inspector-bugs/styles-redesign/tests/style-added-removed-via-js.html 4. Quickly add a "font-family" property to Style Attribute 5. Press Tab to focus on the value field Created attachment 335807 [details]
Patch
Created attachment 335815 [details]
Patch
Fixed a typo.
The focus is still lost (along with uncommitted changes) when an inherited rule changes for the selected element.
Test Case:
<script>
let s = document.createElement("style");
s.innerText = "body { color: red; }";
setInterval(() => {
s.parentNode ? s.remove() : document.head.append(s);
}, 3000);
</script>
<body>
<p>Body text alternates between black and red, every 3 seconds.</p>
</body>
Steps to Reproduce:
1. Inspect <p> element
2. Start editing the Style Attribute
=> Focus and changes lost
Comment on attachment 335815 [details]
Patch
Resetting review based on Matt's comment.
Comment on attachment 335815 [details] Patch I filed a separate issue, since it would require a different kind of a fix. https://bugs.webkit.org/show_bug.cgi?id=183755 Web Inspector: Styles: Don't remove or add style sections while CSS property or selector is focused This problem this patch fixes is that adding/removing a stylesheet resets focus, even if NONE of the rules of that added/removed stylesheet matches the inspected element! Comment on attachment 335815 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=335815&action=review r=me > Source/WebInspectorUI/ChangeLog:11 > + selection, and completion. I think this could be reworded as: Adding or removing a stylesheet causes SpreadsheetRulesStyleDetailsPanel to refresh, triggering a layout of all SpreadsheetCSSStyleDeclarationSection child views. This resets the focus, selection, and auto-completion state. > Source/WebInspectorUI/ChangeLog:13 > + This patch prevents re-layouts of SpreadsheetCSSStyleDeclarationSection when its CSS properties are being edited. And: This patch prevents SpreadsheetCSSStyleDeclarationSection from performing a layout when a property is being edited. > Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js:65 > + // Don't re-layout all properties when one of them is being edited. Full re-layout resets "re-layout" is awkward. What about: // Prevent layout of properties when one of them is being edited. A full layout resets... Created attachment 336169 [details]
Patch
Comment on attachment 336169 [details] Patch Rejecting attachment 336169 [details] from commit-queue. Number of test failures exceeded the failure limit. Full output: http://webkit-queues.webkit.org/results/7046138 Created attachment 336173 [details]
Archive of layout-test-results from webkit-cq-02 for mac-sierra
The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: webkit-cq-02 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 336169 [details] Patch Rejecting attachment 336169 [details] from review queue. nvasilyev@apple.com does not have reviewer permissions according to https://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/contributors.json. - If you do not have reviewer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have reviewer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your reviewer rights. Created attachment 336174 [details]
Patch
Comment on attachment 336174 [details] Patch Clearing flags on attachment: 336174 Committed r229787: <https://trac.webkit.org/changeset/229787> All reviewed patches have been landed. Closing bug. |