Bug 147524 - Web Inspector: Replace "unknown-mac" CSS class with "el-capitan"
Summary: Web Inspector: Replace "unknown-mac" CSS class with "el-capitan"
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nikita Vasilyev
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-08-01 00:40 PDT by Nikita Vasilyev
Modified: 2015-08-04 14:53 PDT (History)
8 users (show)

See Also:


Attachments
Patch (8.78 KB, patch)
2015-08-01 00:44 PDT, Nikita Vasilyev
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Vasilyev 2015-08-01 00:40:59 PDT
"unknown-mac" CSS class was introduced in https://bugs.webkit.org/show_bug.cgi?id=145419, before El Capitan announcement. It shouldn't be called "unknown" anymore.
Comment 1 Nikita Vasilyev 2015-08-01 00:44:28 PDT
Created attachment 258000 [details]
Patch
Comment 2 Timothy Hatcher 2015-08-01 03:04:22 PDT
Comment on attachment 258000 [details]
Patch

Great!
Comment 3 Timothy Hatcher 2015-08-01 03:06:33 PDT
Comment on attachment 258000 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=258000&action=review

> Source/WebInspectorUI/UserInterface/Views/Toolbar.css:161
> +body.el-capitan .toolbar .search-bar > input[type="search"] {

All these rules that don't use :not(.el-cap) will need to change before the next release. Let's fix that in a follow up.
Comment 4 WebKit Commit Bot 2015-08-01 03:55:36 PDT
Comment on attachment 258000 [details]
Patch

Clearing flags on attachment: 258000

Committed r187715: <http://trac.webkit.org/changeset/187715>
Comment 5 WebKit Commit Bot 2015-08-01 03:55:41 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Nikita Vasilyev 2015-08-01 05:25:47 PDT
Comment on attachment 258000 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=258000&action=review

>> Source/WebInspectorUI/UserInterface/Views/Toolbar.css:161
>> +body.el-capitan .toolbar .search-bar > input[type="search"] {
> 
> All these rules that don't use :not(.el-cap) will need to change before the next release. Let's fix that in a follow up.

I'm not following. What will they need to change for?
Comment 7 Timothy Hatcher 2015-08-01 09:32:39 PDT
Comment on attachment 258000 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=258000&action=review

>>> Source/WebInspectorUI/UserInterface/Views/Toolbar.css:161
>>> +body.el-capitan .toolbar .search-bar > input[type="search"] {
>> 
>> All these rules that don't use :not(.el-cap) will need to change before the next release. Let's fix that in a follow up.
> 
> I'm not following. What will they need to change for?

When the system isn't el-capitan for the next release, they will be unknown-mac again, these styles will revert to the old look.

That is why most of these rules are about styling the old (body:not(.el-capitan)), so all future releases keep the new look.
Comment 8 Nikita Vasilyev 2015-08-03 02:47:09 PDT
We currently have 2 versions in our CSS:
— body:not(.el-capitan) for OS X <10.11
— body.el-capitan for OS X 10.11

There is a problem that 10.12, or anything >10.11 for that matter, currently gets "unknown-mac" class and falls into body:not(.el-capitan) selector. We don't want that; we want 10.12 to have El Capitan styles until we introduce a new version.

WebInspector.Platform.version.name is currently used to add a corresponding class name to <body>. What if we do something like this instead:

    if (WebInspector.Platform.version.release <= 10)
        document.body.classList.add("legacy");
    else if (WebInspector.Platform.version.release >= 11)
        document.body.classList.add("latest-mac");

CSS selectors:
— body.legacy 
— body.latest-mac

It would eliminate the need for :not() selector.

Later, when we need to have a new (third) UI version, we would:
— Keep .legacy CSS rules unchanged
— Rename all .latest-mac to .el-capipan
— Write all new rules under .latest-mac prefix

What do you think?
Comment 9 Timothy Hatcher 2015-08-04 14:53:20 PDT
That is fine with me.