Bug 34040 – Web Inspector: Use conditional CSS for defining monospace fonts instead of native code in clients.

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Pavel Feldman]

That would allow inspector to define its looks for platforms and would make sure inspectors on all macs look alike (no matter whether it is WebKit/Safari/Chromium/etc.

[Alexander Pavlov (apavlov)]

Created attachment 47614 [details]
[PATCH] Proposed solution

[Pavel Feldman]

Comment on attachment 47614 [details]
[PATCH] Proposed solution

>  
> -body.platform-windows .source-code {
> -    font-family: Consolas, Lucida Console, monospace;
> +body.platform-mac-tiger .source-code, body.platform-mac-leopard .source-code {
> +    font-size: 10px;
> +    font-family: Monaco, monospace;
> +}

Could you group definitions for monospace and source-code?

>          if (!("_platform" in this))
> -            this._platform = InspectorFrontendHost.platform();
> -
> +            this._detectPlatform();

I liked the lazy init pattern, should detectPlatform return a value?

>          return this._platform;
>      },
>  
> +    _detectPlatform: function()
> +    {

> +
> +        function getOSFromUserAgent(userAgent)
> +        {
> +            if (userAgent) {
> +                var platformInfo = userAgent.match(/\(([^\)]+);\s*([^\)]+);\s*([^\)]+);\s*([^\)]+)\s*\)/);
> +                if (!platformInfo)
> +                    return;
> +                return platformInfo[3].trim();
> +            }
> +            return null;
> +        }
> +

I am not really sure whether this is needed. Why not to match Mac/Windows against original user agent?

[Alexander Pavlov (apavlov)]

Created attachment 47619 [details]
[PATCH] Comments addressed

Comment on attachment 47619 [details]
[PATCH] Comments addressed

Isn’t this going to result in the wrong font size being used on Mac OS X versions later than 10.6?

[WebKit Commit Bot]

Comment on attachment 47619 [details]
[PATCH] Comments addressed

Clearing flags on attachment: 47619

Committed r54001: <http://trac.webkit.org/changeset/54001>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Timothy Hatcher]

(In reply to comment #4)
> (From update of attachment 47619 [details])
> Isn’t this going to result in the wrong font size being used on Mac OS X
> versions later than 10.6?

No.

body.platform-mac-snowleopard .monospace,
body.platform-mac-snowleopard .source-code {
    font-size: 11px;
    font-family: Menlo, monospace;
}

(In reply to comment #7)
> (In reply to comment #4)
> > (From update of attachment 47619 [details] [details])
> > Isn’t this going to result in the wrong font size being used on Mac OS X
> > versions later than 10.6?
> 
> No.
> 
> body.platform-mac-snowleopard .monospace,
> body.platform-mac-snowleopard .source-code {
>     font-size: 11px;
>     font-family: Menlo, monospace;
> }

I was talking about the earlier version of the patch that sent anything above 10.6 to the “Mac unknown” path, which was resulting in 10px.