Fix for incorrect CSS selectors specificity (fixes at least #2531, #1873 and #2621)

[nazar-pc]

As reported in #2531 (while I expect many other similar might be fixed as well).
I've also fixed 2 existing tests where order of styles was hardcoded (it changed a bit now, but preserves priority even better and no other tests affected).
Tested manually under Shady DOM, Shadow DOM and Shadow DOM polyfill.
More tests specific to these improvements will be added soon (now it is time to sleep:)).

[nazar-pc]

Tests are also added (pretty basic, but anyway).

[nazar-pc]

Added fix for #1873 since it touches similar places, so it would be much easier to merge, tests added for this case as well.
One caveat here: @apply location doesn't have any difference, namely:

.green-2 {
  border-top: 2px solid green;
  @apply(--x-overriding);
}

works in the same way as:

.green {
  @apply(--x-overriding);
  border-top: 2px solid green;
}

So, @apply rule kind of pops-up.
I'm not sure if it is according to specs, but implementing it in such way that takes order into account would be MUCH more difficult.
Also thinking logically, if you have local property - it should override one from mixin, otherwise it doesn't really make much sense.

[nazar-pc]

@sorvell, it seems you're the one whom I should ask for review here.

[frostius]

@nazar-pc

So, @apply rule kind of pops-up.
I'm not sure if it is according to specs, but implementing it in such way that takes order into account would be MUCH more difficult.
Also thinking logically, if you have local property - it should override one from mixin, otherwise it doesn't really make much sense.

I would love to see order taken into account

1. it is consistent with current CSS behavior where order drives specificity. Besides being "correct" it would meet the principal of least surprise
2. it allows order to be used to set a default style in case the @apply doesn't. Users of libraries don't always know (or want to know) the details of the library - especially with complex libraries or novice/casual users.
3. If there is to be pain (it's hard), let's keep the pain point as small as possible - in your code in this case :)

Using your example, this allows me to set a default border-top. The @apply may/may not have border styles

.green-2 {
  border-top: 2px solid green;
  @apply(--x-overriding);
}

[nazar-pc]

1. Agree, but I'll need to read through specs draft first, I'll do that a bit later
2. Makes sense
3. It would not be too easy, but I have some thoughts how to make it order-aware, I just need some time for this, stay tuned:)

[TimvdLippe]

Is it possible to include a fix for #2621 too? (if this PR doesn't already fix the problem described) Seems they are closely related.

[nazar-pc]

@JeremybellEU, this PR should fix #2621 already.

[TimvdLippe]

👍 The PR description should probably add "Fixes #2621" to get it in the changelog. Great work @nazar-pc !

[nazar-pc]

Asked about @apply position awareness here: tabatkins/specs#45
No response yet, but since it is much more logical to actually take position into account - I've added that as well in latest commit.

Now this PR seems to be complete and ready to radically improve experience of Polymer users when dealing with CSS mixins/variables 🚀🚀🚀

@mfrost66, give it a try.

P.S. @sorvell, I hope you'll find some time for review in nearest future. Some of my non-merged PRs are waiting from 5th of August till today 😢

[azogheb]

+1
Will be nice to see these styling issues resolved!!

[samccone]

R: @tjsavage for slotting

[dfreedm]

This PR is pretty big, so I'm reviewing it last. I'll do the merge conflict work unless it gets too nasty.

[nazar-pc]

I can rebase it if necessary

[dfreedm]

that would be great :)

[nazar-pc]

Rebased against master

[BLamy]

👍 I can finally stop spamming !important.

[dfreedm]

@sorvell for review

[nazar-pc]

WOW, just simplified it A LOT!
Also added more tests.
Turns out just one if caused all kinds of issues here:)

[sorvell]

Thanks for doing this PR. We were considering solving this issue as in the PR, but we're a little concerned about the performance burden of having excess rules in the custom properties css. I'm going to try to dive into this and if the cost looks low, we'll go ahead and merge this.

Our longer term plan is to explore integrating custom property css together with the rest of the element css, making updates to custom properties via the CSSOM and using a single stylesheet for all of an element's styles. This is fairly involved and will mean more significantly forking the code for Shadow v. Shady DOM.

[nazar-pc]

Rebased against master to resolve merging conflicts, squashed everything into single commit, since history was not useful.

[ebidel]

#2642 is another specificity issue likely fixed by this.

[nazar-pc]

I guess it was a typo, #2642 is this PR's id.

[ebidel]

Yes #2731 is the issue

[frostius]

@ebidel Can you provide a status update?

I'm discouraged by the level of changes since the 1.0 release in regards to styling. Not only have you significantly changed the feature set of 1.x with deprecation of /deep/ and css imports, but the proposed solution (mixins,vars) is buggy and of apparent low priority as evidenced by the age of this PR. The level of effort required on our (users) part to stay up with an every-changing API is non-trivial. More so when the features are either buggy or not well thought out.

Our longer term plan is to explore integrating custom property css together with the rest of the element css, making updates to custom properties via the CSSOM and using a single stylesheet for all of an element's styles. This is fairly involved and will mean more significantly forking the code for Shadow v. Shady DOM

Your statement above implies that there is more significant change planned for the future. My initial enthusiasm for web components is slowly turned into trepidation. A clear statement on the status of styles and roadmap/timeline for a "final" solution would be greatly appreciated. A timely fix for the current bugs surrounding styles would also be wonderful. A caveat emptor on the polymer home page wouldn't be out of order until the team's able to sort this mess out.

[nazar-pc]

@mfrost66, if you don't mind you can use often up to date version of Polymer with bunch of extremely useful patches in JS form (not HTML imports form to speed up things in browsers other than Chromium) here: https://github.com/nazar-pc/CleverStyle-CMS/tree/master/includes/js
There is file a4.polymer-1.2.2+patches.min.js there currently, you can see file history, there is complete list of patches included in commit message.

And yes, I'm very sad with pace of PRs merging too, since most of major issues are in fact already fixed for a long time now, just not upstreamed.

[frostius]

@nazar-pc Thanks, which of the libraries would I need - just the a4.polymer*.js file?

I actually tried to apply your patches locally but ran into the sad fact that polymer doesn't build on windows and I was too flabbergasted to keep trying.

[nazar-pc]

@mfrost66, yes, just that file - it contains Micro, Mini and Standard - full Polymer version that you'd get by including polymer.html.

[frostius]

@nazar-pc
I had expected the styles would penetrate into module content. Is this a bug or is there an alternate way of doing this?

In the example below, "inner content" should be colored red.
Example

test.html

<!DOCTYPE html>
<html>
<head>
  <script charset="utf-8" src="bower_components/webcomponentsjs/webcomponents.js"></script>
  <link rel="import" href="bower_components/polymer/polymer.html">
  <link rel="import" href="my-modules.html">
</head>
<body>
<my-paragraph>
  <my-text>inner content</my-text>
</my-paragraph>
</body>
<script>
</script>
</html>

my-modules.html

<dom-module id="my-text">
  <style>
    :host {
      display: block;
      color:var(--my-text-color,blue);
    }
  </style>
  <template><content></content></template>
  <script> Polymer({ is:'my-text'}) </script>
</dom-module>
<dom-module id="my-paragraph">
  <style>
    :host { --my-text-color:red; }
    :host ::content * { --my-text-color:red; } /*tried this too*/
  </style>
  <template>
    <my-text>Outer content</my-text>
    <content></content>
  </template>
  <script> Polymer({ is:'my-paragraph'}) </script>
</dom-module>

[nazar-pc]

It should not work, you're applying color to my-text host, not its contents. If you have any questions - ask on stackoverflow or somewhere else, please, since it doesn't really relate to this issue directly.

[dfreedm]

This change looks fine to me. @nazar-pc can you rebase this PR?

[frostius]

Hmm, I believe it should in fact work. From polymer styles doc's:

These custom properties can be defined similarly to other standard CSS properties and will inherit from the point of definition down the composed DOM tree, similar to the effect of color and font-family

Perhaps its not related to this PR? I'll post it as a new bug.

[dfreedm]

@nazar-pc scratch that, I'll rebase it since the conflict is with something I wrote :P

[dfreedm]

Merged!

Sorry for the delay, the team has been heads down on some performance work, and I'm helping get everything back on track.