Inline styles are not supported properly anymore

[jogibear9988]

I use the grid polyfill in a polymer component!

but getcomputedstyle always returns "attr(style)"

see also http://stackoverflow.com/questions/34761994/css-grid-errors-using-grid-polyfill

[FremyCompany]

Hi,

Could you please provide a live demo of the issue, either as an upload or as a jsbin/codepen/jsfiddle? Which browser do you use and that reproduces the issue? Did you try other ones?

"attr(style)" is a special value I use in fake css rules to attempt to recognize css inline styles in my getComputedStyle polyfills, but it should not affect the final value of anything, this is just an internal trick. Now, I refactored pretty deeply how I manage css rules internally, I might have made a mistake somewhere that make this leak into the browser.

I would need a test case to investigate, though.

Best regards, Francois

[jogibear9988]

I tried... But I cannot get the Gridpolyfill Execute in JsFiddle!

See: https://jsfiddle.net/vfdq2ghx/2/ or https://jsfiddle.net/vfdq2ghx/3/

[jogibear9988]

Maybe you can help me?

[jogibear9988]

Also without polymer or anything it does not work: https://jsfiddle.net/ftznsfqh/

[FremyCompany]

Thank you so much, your test case looks perfect to showcase the problem.

I would think the issue is that you are using inline styles. I'm pretty sure I got that mostly working at some point, but it was very hacky.

I would recommend you to use css rules and classes in your jsfiddle to apply your styling, and see if that solves the issue.

If inline style is the issue, I'll try to work on it using another quick fix to make it work again. As a general rule of thumb, though, inline style is not supposed to work in my polyfills because there is no good way to detect those easily.

Regarding the Polymer case, I'm pretty sure Polymer must be doing something when cloning/inserting the elements in the the dom which by chance makes the inline style hooks work again, but sadly it seems it is broken anyway which is why you get the error messages.

[jogibear9988]

Maybe we could also test if we switch polymer from shady to shadow dom! If then the polyfill will work! (I dont know if this is posibble at all

[jogibear9988]

Any Idea if this could be solved, or can I help you with something? Maybe you can tell me where in your Code you search for the inline styles?

[FremyCompany]

I'll try to work on that this evening. Stay tuned for more advances on the matter.

[FremyCompany]

Sadly, I think I'll give up on this for today. Had to do other stuff today, which prevented me to have a closer look. That being said, I did think about the problem and know how I want to solve it.

https://github.com/FremyCompany/css-grid-polyfill/blob/master/src/core/css-cascade.js#L763 This is where I track inline styles. Firstly, it should be checked that this is actually invoked for the properties we are looking for. It is very likely it is, but we have to make sure.

Secondly, the inline style should be found there when the analysis for the property is done: https://github.com/FremyCompany/css-grid-polyfill/blob/master/src/core/css-cascade.js#L317

I think the issue is that myStyle['grid-row'] and co do not attempt to parse the style attribute: https://github.com/FremyCompany/css-grid-polyfill/blob/master/src/core/css-cascade.js#L722

That means that while the polyfill actually detects the inline style, it cannot get its value. The fix I have in mind would be to replace this last line so that if the data-style-xxxx attribute does not exist, the code path tries to parse "*{" + element.getAttribute("style") + "}" and check if the resulting rule has any property named "xxxx". If yes, return its value, if not return an empty string.

[FremyCompany]

If you get some time to try this fix, let me know. Otherwise, I keep this on top of my backlog anyway so I could potentially get to it reasonably soon.

[jogibear9988]

The https://github.com/FremyCompany/css-grid-polyfill/blob/master/src/core/css-cascade.js#L713 is never invoked!

[jogibear9988]

I don't know if this is correct, but the conndition in this line: https://github.com/FremyCompany/css-grid-polyfill/blob/master/src/core/css-cascade.js#L500 is not true!

[jogibear9988]

cssCascade.monitoredProperties is a empty object

[jogibear9988]

The onupdate in lin 493 is called before startMonitoringProperties in line 522 is this correct?

[jogibear9988]

I've found out a few other issues:

If you add the polyfill in the head (https://jsfiddle.net/ftznsfqh/2/) then you get a Exception: "Cannot read property style of null" here: https://github.com/FremyCompany/css-grid-polyfill/blob/master/src/css-grid/polyfill.js#L6

If you add it before you add the grid to the dom (https://jsfiddle.net/ftznsfqh/3/) then you get the "INVALID DECLARATION: grid-column/row: attr(style) / attr(style) (invalid token)" Exception

And if you use the first Code (https://jsfiddle.net/ftznsfqh/) where the polyfill is loaded after the grid then the cssCascade.monitoredProperties are empty in line https://github.com/FremyCompany/css-grid-polyfill/blob/master/src/core/css-cascade.js#L496

[jogibear9988]

I tried this:

That means that while the polyfill actually detects the inline style, it cannot get its value. The fix I have in mind would be to replace this last line so that if the data-style-xxxx attribute does not exist, the code path tries to parse "*{" + element.getAttribute("style") + "}" and check if the resulting rule has any property named "xxxx". If yes, return its value, if not return an empty string.

with Example https://jsfiddle.net/ftznsfqh/3/

The code goes throu this line: https://github.com/FremyCompany/css-grid-polyfill/blob/master/src/core/css-cascade.js#L722 for every polyfiled property! But after the first iteration (where css-propertyname is "order"), the style attribute of the parent element does not have the grid attributes any more!

[jogibear9988]

In https://github.com/FremyCompany/css-grid-polyfill/blob/master/src/core/css-cascade.js#L317 if we change to:

 element.attributes['style'].value.split(';').forEach(function (item) { if (item.trim().startsWith(cssPropertyName)) console.log(item.trim().substring(cssPropertyName.length).trim().substring(1).trim())});

Ok, no Console Log, but use the value we find this way??? But why is this needed?

Possible Fix:

https://github.com/FremyCompany/css-grid-polyfill/compare/master...jogibear9988:patch-1

[FremyCompany]

Thank you so much for digging into this!

I think you are totally right about the diagnosis:

1. The polyfill does not seem to see inline styles on elements declared before it is created. I will have to understand why, this seems to be an edge case I don't understand.
2. The polyfill does not seem to get the value of inline styles on elements declared after it is created if they are not in their corresponding data attribute. That was the bug I was suspecting, though your fix isn't good enough to avoid parsing issues in case the style attribute contains comments or invalid declarations. I know how to fix this, though.

I don't have time to review the other bugs you found right now, but I'll definitely fix them too when I get some time. Spending time on my polyfill projects has been more difficult recently, but I value your investment highly so I'll make sure to make fixing these issues a priority.

[jogibear9988]

Maybe I've some more time tomorrow and could dig a little bit more into some issues...

I think the issue, that the polyfill does not work when decleard in the head could also be fixed easily if we when body is null, execute it in window.onload...

[jogibear9988]

any news? or maybe you could help me?

[FremyCompany]

Hi,

Just wanted to touch base since it's been a really long time since I last intervened here.

My job in the Microsoft Edge team takes a huge chunk of my willingness to code, and maintaining my css polyfills does not appeal me as much anymore as it used to, mostly because I don't have any use for them anymore, except as proofs of concept.

That doesn't mean I will stop improving those, but my current thinking is to move them towards a new polyfill framework instead of trying to improve the current one. The current infrastructure will never be efficient enough to empower most websites, and this really bothers me. I have been thinking about how to fix this for some time already, but thanks to the data and patterns I have been reviewing lately as part of my job, I finally found the missing piece today that would enable to build the solution. The new infrastructure I have now in mind would be efficient enough to empower big websites, and would be considerably simpler to maintain and understand than what I have built now.

The system I built and that my polyfills use today was an incredible achievement. I don't think many people in the industry would have believed in this project seriously when I started. Nowadays though, companies like Google have employees working full-time on similar ideas. That means the industry and I am now ready to move to the next level of css polyfilling, and that is a much better use of my free time than maintaining the current level. I cannot say when I will start working on this new backend, but this is definitely a project I want to work on.

Now if you want to dig into the current code, and have any question, I would be glad to answer them, but I will not work on this personally.

---

So, to start this trend, let me answer your latest questions:

1. But why is this needed? --- because the browser does not understand the property we ask him to return, and acts as if we never specified it. Actually, the code I wrote works in IE, which was my test browser, and the reason why I never spotted the mistake.
2. You told me you knew how to fix my patch. How? --- Instead of splitting the style property by ; you should use the cssParser API to parse it as a css stylesheet (by prepending *{ and appending } to the value of the style attribute) then walk the declarations of the first and only rule of the stylesheet to see if any matched. That is more code but would work better if someone wrote style="abc:abc;/*xy;xy;*/yz:yz".

[RichiCoder1]

Random dude here: I look forward this future project if and when it comes to fruition! I didn't realize you worked on the Edge team.

[FremyCompany]

@RichiCoder1: I started only 6 months ago, that's why ;-)

[RichiCoder1]

@FremyCompany Haha, well I wish you the best and hope your time with the Edge team has been good so far! They seem like a great team.

[jogibear9988]

news?

[FremyCompany]

News regarding a new version of the polyfill engine: I have been iterating on my design. I made some progress but didn't decide yet which approach would work best between the two remaining candidates. I will start working on an implementation once I'm confident I'm doing the right thing, and I can assign enough bandwidth to this project.

Just to give some reasonable expectations, I do not expect any new major version before at least February 2017. It is very likely I'll start implementing the new version for real starting December 2016 when I'll have more free time. I will likely do some one-offs and experiments before (actually, the two next weeks should provide me with enough time to start prototyping one of the two candidates; exactly the subsystem of the prototype which would fit in to fix this bug by the way) but porting the entire infrastructure is a huge piece of work that require some dedication I cannot afford at this time.

For reference, here are the two designs I'm currently considering. Both involve some build-time preprocessing that will speed up things on the client-side. It will probably be possible to do this processing client-side but this would totally miss the point.

The first one is the "active intermediary" approach where I take less dependency on the browser than I currently do, and therefore do more of the work myself, and do not feed the browser with things I will take care myself, to avoid as much as possible duplicated work. The advantage is that I'll be forced to make less tradeoffs and polyfill less API (which will improve reliability).

The other "passive listener" design is to keep relying on the browser's own features (and take advantage of the new ones where possible) but add some additional metadata to inform my decision in areas where the browser would otherwise not give me enough information. This will put more burden on the browser yet still incur some tradeoffs (animations, transitions, etc), but would make my client-side code way easier... if it were not for the polyfills I'd have to support. Based on my current investigations, three implementations would be required: Compatible browsers (easy), Internet Explorer (easy), and not-recent Android/iOS/Opera (tough). I haven't quite figured out the story for the last group. I'm sometime afraid it will be required to do much of what I would to for the active solution anyway, which makes me uneasy. Another option would be to drop support for those browsers. Looking at usage stats, I don't feel very confident this is a good option yet. It will become more viable as time goes on, though.

The former is more attractive to me, but I think the latter will be more attractive to most developers out there. At least that is what I'd think.

[jogibear9988]

what will you polyfill on compatible browsers? i think there we dont need it

[FremyCompany]

Grid is still not supported by default in any browser ;) My other polyfill (CSS Regions) would also benefit from these changes of infrastructure if it improves performance, though it does not need them as of right now.

Mainly, this infrastructure is meant to be used to polyfill/try-to-implement new things that do not exist yet, or do not exist everywhere. It is why I built it, more than to mrely build the grid-and-regions polyfills themselves.

I have something specific in mind at this point, but I don't want to talk about it right now -- because now that I work for a browser team, people might (correctly or incorrectly) assume the things I do might have some connection with what my team is doing, and I don't want to enter into mind games at all when I don't think it is necessary ;-)

[jogibear9988]

In webkit nightly and safari tech. preview grid is available unprefixed. So in the next Version it will be there.