Created attachment 363563 [details] [Image] Screenshot of Issue # STEPS TO REPRODUCE: 1. inspect any page with a <canvas> 2. go to the Canvas tab 3. take a recording of any <canvas> on the page 4. wait for the recording to finish processing => "Recording 1" scope bar in the navigation sidebar has no background
It also appears to have broken the Audit tab :(
Created attachment 363571 [details] Patch
Comment on attachment 363571 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363571&action=review rs=me Sounds good to me. > Source/WebInspectorUI/ChangeLog:9 > + use CSS variables instead. Divides the variables into three categories: I only see 2 groups, default and override. I don't see "actual". > Source/WebInspectorUI/UserInterface/Views/AuditTestGroupContentView.css:98 > + vertical-align: -4px; How is this calculated? Just adjusting until it looks nice? > Source/WebInspectorUI/UserInterface/Views/ScopeBar.css:-129 > -.scope-bar > li:matches(.selected, :active) { > - transition-duration: 75ms; > -} Heh, so we aren't transitioning anything?
Comment on attachment 363571 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363571&action=review > Source/WebInspectorUI/UserInterface/Views/ScopeBar.css:60 > + border: 1px solid var(--scope-bar-border-color); Borders are only used in the Audit tab, correct? What about just overriding the width, height, and border properties in AuditTestGroupContentView.css, since it isn't generally used?
Comment on attachment 363571 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363571&action=review >> Source/WebInspectorUI/ChangeLog:9 >> + use CSS variables instead. Divides the variables into three categories: > > I only see 2 groups, default and override. I don't see "actual". The actual is the variable that doesn't have an `-override` or `-default` at the end of it. The idea is that you'd want to set the `-override` variable (e.g. `--scope-bar-text-color-override`) and get the non-`-override` variable (e.g. `--scope-bar-text-color`). >> Source/WebInspectorUI/UserInterface/Views/AuditTestGroupContentView.css:98 >> + vertical-align: -4px; > > How is this calculated? Just adjusting until it looks nice? Yup. I'd like to avoid setting "significant" properties (e.g. `display`) as they can lead to very different behavior. This only affects the <img> (whereas `display` affects the `WI.ScopeBarItem`'s element). >> Source/WebInspectorUI/UserInterface/Views/ScopeBar.css:60 >> + border: 1px solid var(--scope-bar-border-color); > > Borders are only used in the Audit tab, correct? What about just overriding the width, height, and border properties in AuditTestGroupContentView.css, since it isn't generally used? I'd rather not, as that means that if we did add a border in the future, it would break the audit tab. That's exactly what happened to cause this bug. We should strive to be as flexible as possible with our general UI views so that they can handle multiple things at once (or at the very least have them be written in such a way so as to make sure that changes don't break varying uses). >> Source/WebInspectorUI/UserInterface/Views/ScopeBar.css:-129 >> -} > > Heh, so we aren't transitioning anything? Not that I could tell. The only one I did find was the console's filters, but that uses an animation.
Created attachment 363591 [details] Patch Resetting back to r? since I did some additional fixes with the console
Comment on attachment 363591 [details] Patch Attachment 363591 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/11372705 New failing tests: fast/images/imagemap-polygon-focus-ring.html fast/scrolling/ios/hit-testing-iframe-003.html
Created attachment 363608 [details] Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 363608 [details] Archive of layout-test-results from ews125 for ios-simulator-wk2 Unrelated test failures.
Comment on attachment 363591 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363591&action=review r=me > Source/WebInspectorUI/UserInterface/Views/LogContentView.js:96 > + indicatorElement.classList.add("indicator"); Nit:`elem.className = "indicator"` is more common and faster for the simple case where we just created the element and are setting classes. > Source/WebInspectorUI/UserInterface/Views/MultipleScopeBarItem.js:178 > // Clicking when selected will cause the menu it appear instead. Fix the typo in this comment while you're in the area. "menu it appear" => "menu to appear" > Source/WebInspectorUI/UserInterface/Views/MultipleScopeBarItem.js:181 > + if (this._element.classList.contains("selected")) { > + let newEvent = new event.constructor(event.type, event); > + newEvent.__original = event; This should really be its own separate change.
Comment on attachment 363591 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363591&action=review >> Source/WebInspectorUI/UserInterface/Views/MultipleScopeBarItem.js:181 >> + newEvent.__original = event; > > This should really be its own separate change. I would tend to agree, but in this case, since I was already fixing so much of the CSS for `WI.ScopeBar`, I felt that it would make sense.
Created attachment 363895 [details] Patch
Comment on attachment 363895 [details] Patch Clearing flags on attachment: 363895 Committed r242604: <https://trac.webkit.org/changeset/242604>
All reviewed patches have been landed. Closing bug.
<rdar://problem/48683234>