Skip to content

Set Edge's expected support for clearCloneStyle to true #2857

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
zackhall wants to merge 1 commit into from
Closed

Set Edge's expected support for clearCloneStyle to true #2857

zackhall wants to merge 1 commit into from

Conversation

@zackhall
Copy link
Contributor

@zackhall zackhall commented Jan 22, 2016

The Microsoft Edge team noticed the following test failure in Edge from test/unit/support.js: "Verify that support tests resolve as expected per browser." If you're running the tests locally, I isolated the test with the query param ?testId=e1b3f3c8.

Previous versions of Edge and IE had a bug in which cloning an element and then clearing the cloned element's style would also clear the original element's style. See #886 for an excellent and thorough analysis.

Turns out, somewhere along the line Edge fixed this bug but the unit tests expect Edge to fail still.

I've copied the code from support.js#L20 that tests the browser for this bug into a jsbin snippet. If you run it in Edge, you can now see that jQuery determines $.support.clearCloneStyle == true.

@mention-bot
Copy link

mention-bot commented Jan 22, 2016

By analyzing the blame information on this pull request, we identified @mgol, @markelog and @gibson042 to be potential reviewers

@mgol
Copy link
Member

mgol commented Jan 22, 2016

This will break our CI as it runs on Edge 12. In which version of Edge has this been fixed? This will need version branching like what we had for Chrome some time ago, see 48a3549.

@dmethvin
Copy link
Member

dmethvin commented Jan 22, 2016

By the way @zackhall , glad to see this bug was fixed, it was a bizarre one!

@zackhall
Copy link
Contributor Author

zackhall commented Jan 22, 2016

@dmethvin: Yeah, when I was reading back through the history of the old bug it got more and more odd.

@mgol: I'll try to pinpoint which specific version of Edge fixed this. It is passing on my current Edge browser which is Edge v25.11073, EdgeHTML v13.11073.

@mgol
Copy link
Member

mgol commented Jan 22, 2016

@zackhall Is this a stable EdgeHTML 13 version that was released to the public or a newer build? If the stable one then branching at version >= 13 should be enough.

Thanks!

@markelog
Copy link
Member

markelog commented Jan 25, 2016

Relevant - #1036

See #886 for an excellent and thorough analysis.

"Thank you" like that, shouldn't go unnoticed! So thank you @elijahmanor!

@zackhall
Copy link
Contributor Author

zackhall commented Jan 25, 2016

The currently released, stable build of Edge is 13.10586 and we've verified that clearCloneStyle is supported on that build. I will update the PR to branch at version >= 13 as suggested.

@zackhall
Copy link
Contributor Author

zackhall commented Jan 25, 2016

Does the CLA signing take some time to propogate?

I just signed the CLA on the site, and it still says author has not signed the CLA. I also tried using the commit amend strategy on the CLA page.

@mgol
Copy link
Member

mgol commented Jan 25, 2016

@zackhall You need to use the same email address to sign the CLA that you use in commits; they're different now. I see two signatures of you with a Gmail address and these commits use outlook.com.

@mgol
Copy link
Member

mgol commented Jan 27, 2016

LGTM now, thanks!

@mgol mgol added this to the 3.0.0 milestone Jan 27, 2016
@mgol mgol self-assigned this Jan 27, 2016
@mgol mgol closed this in 28f0329 Jan 27, 2016
mgol pushed a commit that referenced this pull request Feb 13, 2016
@zackhall @mgol
Tests: Set Edge's expected support for clearCloneStyle to true
d230d51 
This is done for a version 13 or newer as the bug still exists in Edge
12.

(cherry-picked from 28f0329)

Closes gh-2857
mgol pushed a commit that referenced this pull request Feb 13, 2016
@zackhall @mgol
Tests: Set Edge's expected support for clearCloneStyle to true
e456c30 
This is done for a version 13 or newer as the bug still exists in Edge
12.

(cherry-picked from 28f0329)

Closes gh-2857
@mgol mgol modified the milestones: 1.12.0/2.2.0, 3.0.0 Mar 6, 2016
@mgol
Copy link
Member

mgol commented Mar 6, 2016

This has been backported to 1.12.0 & 2.2.0 so I changed the milestone (it used to say 3.0.0).

@zackhall
Copy link
Contributor Author

zackhall commented Mar 7, 2016

Thanks for the update, @mgol.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

@mgol mgol

Labels

None yet

Milestone

1.12.0/2.2.0

Development

Successfully merging this pull request may close these issues.

6 participants

@zackhall @mention-bot @mgol @dmethvin @markelog @jquerybot