These look heavy for their intended look. <rdar://problem/17731035>
Created attachment 235156 [details] [PATCH] Attempted fix.
Comment on attachment 235156 [details] [PATCH] Attempted fix. View in context: https://bugs.webkit.org/attachment.cgi?id=235156&action=review This looks good, Tim may be better. > Source/WebInspectorUI/UserInterface/Base/ImageUtilities.js:27 > +const _imageStorageFormatVersion = 2; Maybe we can avoid bumping the global version and make generateColoredImagesForCSS do imageVersion magic like the other path.
I think we can use that computed version number as a default for all ImageUtilities.js images unless one is specified in `specifications` and passed in as a parameter for a particular image.
Created attachment 235160 [details] [PATCH] Attempted fix.
Comment on attachment 235160 [details] [PATCH] Attempted fix. View in context: https://bugs.webkit.org/attachment.cgi?id=235160&action=review > Source/WebInspectorUI/UserInterface/Base/Main.js:192 > + if (versionMatch && versionMatch[1].indexOf("+") !== -1 && document.styleSheets.length < 10) { This seems like it should move to Platform.js too.
Comment on attachment 235160 [details] [PATCH] Attempted fix. View in context: https://bugs.webkit.org/attachment.cgi?id=235160&action=review r=me. Have you tested on a Retina machine? > Source/WebInspectorUI/UserInterface/Base/Platform.js:26 > +// Add global platform information. Nit: Unnecessary comment. Maybe this could go in the block below though. > Source/WebInspectorUI/UserInterface/Base/Platform.js:29 > + codeName: "", Nit: Discussed on IRC, making this version.name sounds better! > Source/WebInspectorUI/UserInterface/Base/Platform.js:43 > + toString: function() > + { > + return this.base + "." + this.version; > + } > + }, > + toString: function() > + { > + return this.name; > + } Nit: Unnecessary toStrings. The only people that will debug this are us, and we can just inspect the object or JSON.stringify it. > Source/WebInspectorUI/UserInterface/Base/Platform.js:65 > +var isLegacyMacOS = false; > +var osVersionMatch = / Mac OS X (\d+)_(\d+)/.exec(navigator.appVersion); > +if (osVersionMatch && osVersionMatch[1] === "10") { > + WebInspector.Platform.version.base = 10; > + switch(osVersionMatch[2]) { > + case "10": > + WebInspector.Platform.codeName = "yosemite"; > + WebInspector.Platform.version.release = 10; > + break; > + case "9": > + WebInspector.Platform.codeName = "mavericks"; > + WebInspector.Platform.version.release = 9; > + WebInspector.Platform.isLegacyMacOS = true; > + break; > + case "8": > + WebInspector.Platform.codeName = "mountain-lion"; > + WebInspector.Platform.version.release = 8; > + WebInspector.Platform.isLegacyMacOS = true; > + } > +} I cringe a bit to have these in the global namespace. I suggest we wrap this work in an anonymous block: (function() { ... })(); Like the Load* files.
Created attachment 235231 [details] [PATCH] Attempted fix, review feedback.
Comment on attachment 235231 [details] [PATCH] Attempted fix, review feedback. View in context: https://bugs.webkit.org/attachment.cgi?id=235231&action=review > Source/WebInspectorUI/UserInterface/Base/Platform.js:60 > + WebInspector.Platform.isLegacyMacOS = true; Nit: Would be nice to put a break here even though it is the last.
Created attachment 235232 [details] [PATCH] Attempted fix, review feedback + 1.
Comment on attachment 235232 [details] [PATCH] Attempted fix, review feedback + 1. Clearing flags on attachment: 235232 Committed r171312: <http://trac.webkit.org/changeset/171312>
All reviewed patches have been landed. Closing bug.