fix: webkit properties not being recognized #112

eps1lon · 2020-01-23T17:48:32Z

Seems like they were never tested. I'm not sure if the two added tests can be fixed with independent commits.

codecov-io · 2020-01-23T17:49:59Z

Codecov Report

Merging #112 into master will decrease coverage by 0.31%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #112      +/-   ##
==========================================
- Coverage   37.39%   37.07%   -0.32%     
==========================================
  Files          87       87              
  Lines        1182     1176       -6     
  Branches      227      225       -2     
==========================================
- Hits          442      436       -6     
  Misses        633      633              
  Partials      107      107

Impacted Files	Coverage Δ
lib/allWebkitProperties.js	`100% <100%> (ø)`	⬆️
lib/parsers.js	`80.35% <0%> (-0.3%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 62b3b1b...c01df24. Read the comment docs.

jsakas · 2020-01-23T17:51:18Z

@eps1lon is this a regression we missed in the last PR that ended up being 2.2.0 - or new functionality?

eps1lon · 2020-01-23T17:51:50Z

scripts/generate_implemented_properties.js

-
+  .map(propertyFile => camelToDashed(propertyFile.replace('.js', '')))
+  .map(property => {
+    const isVendorSpecific = /^(o|moz|ms|webkit)-/.test(property);


Sneaking in support for other vendor prefixes. I'm not sure if you're opinionated about other vendor prefixes. I'm looking at this purely from a testing perspective where I'd like to see support for all vendor prefixes. Otherwise it's hard to write tests that cover all browsers at once.

The real bug is in:

cssstyle/lib/parsers.js

Line 712 in 62b3b1b

var first_segment = /^\([^-]\)-/;

It should be:

var first_segment = /^([^-]+)-/;

That’s what I’m doing in #116.

P.S.: JSDOM projects don't use the Angular commit style.

eps1lon · 2020-01-23T17:55:24Z

@eps1lon is this a regression we missed in the last PR that ended up being 2.2.0 - or new functionality?

Tried the tests in 2.1.0 (git checkout v2.1.0) and they were failing as well. I couldn't find any tests related to webkit properties. I can try to create some tests related to webkit properties that are passing in 2.1.0.

domenic · 2020-01-23T18:38:17Z

Only webkit properties are in the spec; other engine prefixes should not be added.

eps1lon · 2020-01-23T18:58:26Z

Only webkit properties are in the spec; other engine prefixes should not be added.

This statement clashes with

cssstyle/lib/allExtraProperties.js

Lines 4 to 5 in ebd2dab

    
            * This file contains all implemented properties that are not a part of any 
        
            * current specifications or drafts, but are handled by browsers nevertheless.

The changes to the test code relate to changes in JSDOM that come with Jest 25: * Several JSDOM workarounds are no longer needed. * Several tests made assertions to match incorrect JSDOM behavior (e.g. setAttribute calls) that JSDOM has now patched to match browsers. * https://codesandbox.io/s/resets-value-of-datetime-input-to-fix-bugs-in-ios-safari-1ppwh * JSDOM no longer triggers default actions when dispatching click events. * https://codesandbox.io/s/beautiful-cdn-ugn8f * JSDOM fixed (jsdom/jsdom#2700) a bug so that calling focus() on an already focused element does not dispatch a FocusEvent. * JSDOM now supports passive events. * JSDOM has improved support for custom CSS properties. * But requires jsdom/cssstyle#112 to land to support webkit prefixed properties.

eps1lon · 2020-03-13T14:08:51Z

@jsakas Is there anything I can do to get this merged?

This reverts commit cf00812. The changes to the test code relate to changes in JSDOM that come with Jest 25: * Several JSDOM workarounds are no longer needed. * Several tests made assertions to match incorrect JSDOM behavior (e.g. setAttribute calls) that JSDOM has now patched to match browsers. * https://codesandbox.io/s/resets-value-of-datetime-input-to-fix-bugs-in-ios-safari-1ppwh * JSDOM no longer triggers default actions when dispatching click events. * https://codesandbox.io/s/beautiful-cdn-ugn8f * JSDOM fixed (jsdom/jsdom#2700) a bug so that calling focus() on an already focused element does not dispatch a FocusEvent. * JSDOM now supports passive events. * JSDOM has improved support for custom CSS properties. * But requires jsdom/cssstyle#112 to land to support webkit prefixed properties.

jsakas · 2020-03-30T22:26:41Z

@eps1lon sorry for the delay here. Are you able to resolve the conflict for me? Also, based on @domenic's comment I think only webkit should be supported at this time.

eps1lon · 2020-03-31T09:23:05Z

Are you able to resolve the conflict for me?

Sure thing.

Also, based on @domenic's comment I think only webkit should be supported at this time.

Personally I would prefer if this also handled other prefixed properties. This is in line with

cssstyle/lib/allExtraProperties.js

Lines 4 to 5 in ebd2dab

    
            * This file contains all implemented properties that are not a part of any 
        
            * current specifications or drafts, but are handled by browsers nevertheless.

But I understand if you want to change that stance to "we only support properties in the spec".

ExE-Boss · 2020-04-01T16:38:47Z

scripts/generate_implemented_properties.js

-
+  .map(propertyFile => camelToDashed(propertyFile.replace('.js', '')))
+  .map(property => {
+    const isVendorSpecific = /^(o|moz|ms|webkit)-/.test(property);


The real bug is in:

cssstyle/lib/parsers.js

Line 712 in 62b3b1b

var first_segment = /^\([^-]\)-/;

It should be:

var first_segment = /^([^-]+)-/;

That’s what I’m doing in #116.

P.S.: JSDOM projects don't use the Angular commit style.

nyroDev · 2023-11-23T14:51:20Z

Hi @eps1lon, do you intend to fix conflict for this pull request?
I encounter an error with a webkit CSS property not being recognized correctly, and this issue should fix it.
I you're not willing to do it, I'll be happy to do it.

What I also noticed is that webkit specific value are accessible through 2 names:
webkitTextFillColor and WebkitTextFillColor

I'm currently not sure if this PR will implement both of them (test case looks like it is)

nyroDev · 2023-11-23T15:01:49Z

BTW allProperties define only 1 webkit property alone, -webkit-line-clamp
I think this PR should remove this line, introduced lately on January

eps1lon · 2023-11-27T17:32:58Z

Not working on this one at the moment

eps1lon added 2 commits January 23, 2020 18:46

test: add webkit properties

dd540ce

fix: webkit properties not being recognized

9150db8

eps1lon commented Jan 23, 2020

View reviewed changes

eps1lon mentioned this pull request Jan 23, 2020

Jest 25 SimenB/react#837

Merged

SimenB mentioned this pull request Jan 24, 2020

chore: upgrade to jest 25 facebook/react#17896

Merged

eps1lon added 2 commits March 31, 2020 11:26

Merge branch 'master' into fix/webkit

c01df24

chore: format code

0f83daf

ExE-Boss suggested changes Apr 1, 2020

View reviewed changes

ExE-Boss mentioned this pull request Apr 1, 2020

Migrate CSSStyleDeclaration to WebIDL2JS #116

Open

nyroDev mentioned this pull request Nov 23, 2023

"webkit-*" vs "-webkit-*" #70

Open

nyroDev mentioned this pull request Nov 26, 2023

Fix webkit properties #166

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: webkit properties not being recognized #112

fix: webkit properties not being recognized #112

eps1lon commented Jan 23, 2020

codecov-io commented Jan 23, 2020 •

edited

Loading

jsakas commented Jan 23, 2020

eps1lon Jan 23, 2020

ExE-Boss Apr 1, 2020 •

edited

Loading

eps1lon commented Jan 23, 2020

domenic commented Jan 23, 2020

eps1lon commented Jan 23, 2020

eps1lon commented Mar 13, 2020

jsakas commented Mar 30, 2020

eps1lon commented Mar 31, 2020

ExE-Boss Apr 1, 2020 •

edited

Loading

nyroDev commented Nov 23, 2023 •

edited

Loading

nyroDev commented Nov 23, 2023

eps1lon commented Nov 27, 2023

fix: webkit properties not being recognized #112

Are you sure you want to change the base?

fix: webkit properties not being recognized #112

Conversation

eps1lon commented Jan 23, 2020

codecov-io commented Jan 23, 2020 • edited Loading

Codecov Report

jsakas commented Jan 23, 2020

eps1lon Jan 23, 2020

Choose a reason for hiding this comment

ExE-Boss Apr 1, 2020 • edited Loading

Choose a reason for hiding this comment

eps1lon commented Jan 23, 2020

domenic commented Jan 23, 2020

eps1lon commented Jan 23, 2020

eps1lon commented Mar 13, 2020

jsakas commented Mar 30, 2020

eps1lon commented Mar 31, 2020

ExE-Boss Apr 1, 2020 • edited Loading

Choose a reason for hiding this comment

nyroDev commented Nov 23, 2023 • edited Loading

nyroDev commented Nov 23, 2023

eps1lon commented Nov 27, 2023

codecov-io commented Jan 23, 2020 •

edited

Loading

ExE-Boss Apr 1, 2020 •

edited

Loading

ExE-Boss Apr 1, 2020 •

edited

Loading

nyroDev commented Nov 23, 2023 •

edited

Loading