.
Created attachment 251611 [details] Patch
Created attachment 251612 [details] Screenshot
Comment on attachment 251611 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=251611&action=review Looks good! r=me > Source/WebInspectorUI/UserInterface/Base/Main.js:251 > + this._closeToolbarButton = new WebInspector.ControlToolbarItem("dock-close", WebInspector.UIString("Close"), platformImagePath("Close.svg"), 16, 14); Screenshot of an attached case? > Source/WebInspectorUI/UserInterface/Base/Main.js:276 > + this.notifications.addEventListener(WebInspector.Notification.PageArchiveStarted, this._pageArchiveStarted, this); > + this.notifications.addEventListener(WebInspector.Notification.PageArchiveEnded, this._pageArchiveEnded, this); Technically since WebInspector is the one dispatching it and since it is the only listener, we could drop the events and just inline the handlers at the dispatch sites. > Source/WebInspectorUI/UserInterface/Base/Main.js:1322 > + WebInspector.archiveMainFrame(); Normally we use "this" inside WebInspector static functions. So this.archiveMainFrame(). > Source/WebInspectorUI/UserInterface/Base/Main.js:1333 > + PageAgent.reload(window.event ? window.event.shiftKey : false); Heh, this shouldn't need to use window.event if it has an event parameter passed in. This should probably drop the comment too: var ignoreCache = event ? event.shiftKey : false; PageAgent.reload(ignoreCache); > Source/WebInspectorUI/UserInterface/Images/Network.svg:1 > +<?xml version="1.0" encoding="utf-8"?> Unused at the moment? Fine to add, or should it be added when it gets used? > Source/WebInspectorUI/UserInterface/Views/ActivateButtonToolbarItem.js:52 > + if (!newLabel || !this._labelElement) Should "get label()" also if check this._labelElement then? Maybe it would be simpler to just always create the element, even if it isn't shown at the moment. > Source/WebInspectorUI/UserInterface/Views/Toolbar.js:26 > +WebInspector.Toolbar = function(element, navigationItems, dontAllowModeChanges) { This boolean may read slightly better as "disallowModeChanges". Or, since this is the only toolbar and thus only one call site flips it to be "allowModeChanges". > Source/WebInspectorUI/UserInterface/Views/Toolbar.js:41 > + this._centerLeftSectionElement.className = WebInspector.Toolbar.ItemSectionStyleClassName + " " + WebInspector.Toolbar.CenterLeftItemSectionStyleClassName; Probably cleaner with classList: this._centerLeftSectionElement.classList.add(WebInspector.Toolbar.ItemSectionStyleClassName, WebInspector.Toolbar.CenterLeftItemSectionStyleClassName); > Source/WebInspectorUI/UserInterface/Views/Toolbar.js:49 > + this._centerRightSectionElement.className = WebInspector.Toolbar.ItemSectionStyleClassName + " " + WebInspector.Toolbar.CenterRightItemSectionStyleClassName; Ditto
r183322 – r183342 http://trac.webkit.org/log/?revs=183322-183342