Docs:Tests: Update IE/Edge-related support comments & tests #3661
Conversation
|
@mgol, thanks for your PR! By analyzing the history of the files in this pull request, we identified @markelog, @gibson042 and @gnarf to be potential reviewers. |
test/unit/css.js
Outdated
| @@ -1323,11 +1323,11 @@ QUnit.test( "Keep the last style if the new one isn't recognized by the browser | |||
| assert.equal( el.css( "position" ), "absolute", "The old style is kept when setting an unrecognized value" ); | |||
| el = jQuery( "<div></div>" ).css( "position", "absolute" ).css( "position", " " ); | |||
|
|
|||
| // Support: Edge 14 | |||
| // Support: Edge 14-15 | |||
There was a problem hiding this comment.
Isn't it should be the opposite – https://en.wikipedia.org/wiki/Dash#Ranges_of_values
There was a problem hiding this comment.
Those are version numbers, not numeric values, we use U+002D HYPHEN-MINUS (which also indicates subtraction), not U+2013 EN DASH, and this is a formal support comment, not English prose.
There was a problem hiding this comment.
Pretty sure that version numbers are numeric :), even if they have designed names, at least according to the semver or any other number schemas really, since i'm not sure how you can describe a range without numeric values.
Most of the text style guides like APA & AMA, which is hardly an "English prose", use variation of dash without the spaces, even HYPHEN-MINUS (specifically "HYPHEN" btw, not "HYPEN") and "EN DASH" are:
equally "correct", and each is the preferred style in some style guides
but always, always without a spaces.
Rules you devised in that comment are great, they are just deviate from most of guides for natural languages (specifically including technical documents) around the world (not only in English actually). I don't think we need to invent a new language or text style guide for this, just to use already existing one.
Like if you invent formal specification to illustrate your point, it doesn't make it widely acceptable idea. Wicked good way to present your point though :).
Instead, I would prefer to use something that everybody use, if you don't like form of dashes used in academic papers (or in typography as a whole), we can replace it with the word "to" with spaces like in SI guide of National Institute of Standards and Technology, which bears insignia of U.S. Department of Commerce
There was a problem hiding this comment.
Pretty sure that version numbers are numeric :), even if they have designed names, at least according to the semver or any other number schemas really, since i'm not sure how you can describe a range without numeric values.
A semantic version uses numbers, but is not itself numeric. And notably, it can include a hyphen (spelling fixed above, thanks), e.g. "1.0.0-0.3.7".
Most of the text style guides like APA & AMA, which is hardly an "English prose", use variation of dash without the spaces, even HYPHEN-MINUS (specifically "HYPHEN" btw, not "HYPEN") and "EN DASH" are:
equally "correct", and each is the preferred style in some style guides
but always, always without a spaces.
That works for them, but is just not acceptable when it can introduce ambiguity. 1.0.0 - 1.1.0-3.1.4 is a valid version range, but take away the spaces and its meaning is no longer clear.
Rules you devised in that comment are great, they are just deviate from most of guides for natural languages (specifically including technical documents) around the world (not only in English actually). I don't think we need to invent a new language or text style guide for this, just to use already existing one.
Instead, I would prefer to use something that everybody use, if you don't like form of dashes used in academic papers (or in typography as a whole), we can replace it with the word "to" with spaces like in SI guide of National Institute of Standards and Technology, which bears insignia of U.S. Department of Commerce
Those are fine opinions, and you can incorporate them in a proposed fix for jquery/contribute.jquery.org#95 if you like. But for this PR, consistency dictates the inclusion of spaces, as is the case in the other support comments.
There was a problem hiding this comment.
I'm asking who this would look like Chrome 1.0.0 - 1.1.0-3.1.4 or something? And I don't get what that supposed to mean, Chrome had an issue in 1.0.0 and then issue resurfaced in 1.1.0 and continue until 3.1.4?
Then why not Chrome 1.0.0 & Chrome 1.1.0-3.1.4?
There was a problem hiding this comment.
If yes, then this already solved with SI style, since they also were concern with negative values like -30--15, it's also looks confusing, this why I proposed to use to as they do
There was a problem hiding this comment.
No, that would mean the bug exists at least from version 1.0.0 to version 1.1.0-3.1.4. There is no such thing as version 3.1.4 in this example.
There was a problem hiding this comment.
This is not the proper place to discuss these issues. If you want to raise an issue with the current syntax, comment on jquery/contribute.jquery.org#95. I'm just following what we use everywhere else right now.
There was a problem hiding this comment.
This is not the proper place to discuss these issues. If you want to raise an issue with the current syntax, comment on jquery/contribute.jquery.org#95
I believe I already did, this is why I raised this question here
I'm just following what we use everywhere else right now.
I'm not asking you to change anything
If you want to raise an issue with the current syntax
There is no current syntax, there is no format for it, it's just something we might use, this is why I created jquery/contribute.jquery.org#95 in the first place
test/unit/css.js
Outdated
| // Edge collapses whitespace-only values when setting a style property and | ||
| // there is no easy way for us to work around it. Just skip the test there | ||
| // and hope for the better future. | ||
| if ( /edge\//i.test( navigator.userAgent ) ) { | ||
| if ( /\bedge\//i.test( navigator.userAgent ) ) { | ||
| assert.ok( true, "Skipped (Edge 14 handles whitespace-only values incorrectly)" ); |
There was a problem hiding this comment.
Please pull this out into its own QUnit[ /\bedge\//i.test( navigator.userAgent ) ? "skip" : "test" ].
|
@gibson042 comments addressed. |
|
I'd like to land it today evening. |
Summary
This PR updates IE/Edge-related support comments (to e.g. update their status for Edge 15) and tweaks some tests that had to be relaxed in the past - most notably smaller precision in some dimensions tests.
Checklist
New tests have been added to show the fix or feature worksIf needed, a docs issue/PR was created at https://github.com/jquery/api.jquery.com