Bug 34469 - Web Inspector: Separate OS type from OS version (WebInspector.platform)
Summary: Web Inspector: Separate OS type from OS version (WebInspector.platform)
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexander Pavlov (apavlov)
Depends on:
Reported: 2010-02-02 03:15 PST by Alexander Pavlov (apavlov)
Modified: 2010-02-02 09:10 PST (History)
7 users (show)

See Also:

[PATCH] Suggested solution (7.46 KB, patch)
2010-02-02 08:05 PST, Alexander Pavlov (apavlov)
pfeldman: review+
pfeldman: commit-queue-
Details | Formatted Diff | Diff
[PATCH] Comments addressed (7.63 KB, patch)
2010-02-02 08:43 PST, Alexander Pavlov (apavlov)
pfeldman: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Pavlov (apavlov) 2010-02-02 03:15:43 PST
Currently the WebInspector.platform value holds both the OS type (mac, windows, etc.) and the OS version (vista-or-later, leopard, etc.)
This complicates the stylesheet rules and results in a lot of duplication. This should be split up into two properties, say, WebInspector.osType and WebInspector.osVersion.
Comment 1 Alexander Pavlov (apavlov) 2010-02-02 08:05:58 PST
Created attachment 47931 [details]
[PATCH] Suggested solution
Comment 2 Pavel Feldman 2010-02-02 08:27:55 PST
Comment on attachment 47931 [details]
[PATCH] Suggested solution

> +body.detached.platform-snowleopard #toolbar {

I'd suggest to rename "platform-snowleopard" to "platform-mac-snowleopard" or "mac-snowleopard"

> +                return WebInspector.PlatformFlavor.WindowsVistaOrLater;

We don't seem to be consistent. We have vista or later, but don't have snow leopard or later. Should Vista imply newer platforms?
Comment 3 Alexander Pavlov (apavlov) 2010-02-02 08:43:45 PST
Created attachment 47936 [details]
[PATCH] Comments addressed
Comment 4 Pavel Feldman 2010-02-02 08:48:14 PST
Comment on attachment 47936 [details]
[PATCH] Comments addressed

> +        this._isMac = (WebInspector.platform === "mac");

No need for () here.
Comment 5 Alexander Pavlov (apavlov) 2010-02-02 09:10:52 PST
"(...)" fixed.

Committing to http://svn.webkit.org/repository/webkit/trunk ...
        M       WebCore/ChangeLog
        M       WebCore/inspector/front-end/InspectorBackendStub.js
        M       WebCore/inspector/front-end/InspectorFrontendHostStub.js
        M       WebCore/inspector/front-end/inspector.css
        M       WebCore/inspector/front-end/inspector.js
Committed r54240