Synchronize use of document globals #2133

Closed
gibson042 opened this Issue Mar 9, 2015 · 10 comments

Comments

Projects
None yet
4 participants
@gibson042
Member

gibson042 commented Mar 9, 2015

Changes from 76df9e4 (in particular, definition and use of the document and documentElement variables) never made it to the compat branch, introducing what I feel to be a needless divergence and source of merge conflicts.

@mzgol: agree?

@gibson042 gibson042 added the 1.x-only label Mar 9, 2015

@gibson042 gibson042 added this to the 3.0.0 milestone Mar 9, 2015

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Mar 9, 2015

Member

I mostly didn't backport it because this is only required for non-browser environments which we don't support in the compat line. But I have nothing against porting them if that makes cherry-picking easier.

Note, though, that on compat we have a different JSHint config that assumes the browser environment; the one on master only declares window & timer functions.

Member

mgol commented Mar 9, 2015

I mostly didn't backport it because this is only required for non-browser environments which we don't support in the compat line. But I have nothing against porting them if that makes cherry-picking easier.

Note, though, that on compat we have a different JSHint config that assumes the browser environment; the one on master only declares window & timer functions.

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Jul 27, 2015

Member

I'll create a document variable as part of #2501 but documentElement doesn't seem worth it - on master it's mostly used in selector-native which is not supported on compat & support tests which use document.body, not document.documentElement on compat.

Member

mgol commented Jul 27, 2015

I'll create a document variable as part of #2501 but documentElement doesn't seem worth it - on master it's mostly used in selector-native which is not supported on compat & support tests which use document.body, not document.documentElement on compat.

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Jul 27, 2015

Member

support tests which use document.body, not document.documentElement on compat.

BTW, it's worth checking if #1342 (comment) still applies or maybe we could switch from body to documentElement for support tests even on compat.

Member

mgol commented Jul 27, 2015

support tests which use document.body, not document.documentElement on compat.

BTW, it's worth checking if #1342 (comment) still applies or maybe we could switch from body to documentElement for support tests even on compat.

@mgol mgol assigned mgol and unassigned timmywil Jul 27, 2015

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Jul 27, 2015

Member

It seems we can switch from body to documentElement on compat, support tests work fine in IE8 before doc ready. :) PR coming.

EDIT: issue created: #2502.

Member

mgol commented Jul 27, 2015

It seems we can switch from body to documentElement on compat, support tests work fine in IE8 before doc ready. :) PR coming.

EDIT: issue created: #2502.

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Jul 28, 2015

Member

PR: #2504.

Member

mgol commented Jul 28, 2015

PR: #2504.

@mgol mgol added CSS Core labels Jul 28, 2015

mgol added a commit to mgol/jquery that referenced this issue Jul 28, 2015

mgol added a commit to mgol/jquery that referenced this issue Jul 28, 2015

mgol added a commit to mgol/jquery that referenced this issue Aug 16, 2015

@markelog

This comment has been minimized.

Show comment
Hide comment
@markelog

markelog Sep 7, 2015

Member

Landed via 04ec688

Member

markelog commented Sep 7, 2015

Landed via 04ec688

@markelog markelog closed this Sep 7, 2015

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Nov 3, 2015

Member

I'd rather not backport it to 1.12 as there is way more compat code there and it may pose problems for Node.js.

Member

mgol commented Nov 3, 2015

I'd rather not backport it to 1.12 as there is way more compat code there and it may pose problems for Node.js.

@mgol mgol removed the Has Pull Request label Nov 5, 2015

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Nov 9, 2015

Member

This is a wontfix now since it was done in connection with #2501.

Member

mgol commented Nov 9, 2015

This is a wontfix now since it was done in connection with #2501.

@mgol mgol removed this from the 3.0.0 milestone Nov 9, 2015

@mgol mgol added the wontfix label Nov 9, 2015

@markelog

This comment has been minimized.

Show comment
Hide comment
@markelog

markelog Dec 22, 2015

Member

Since 1.12 will be released, i will remove wontfix label

Member

markelog commented Dec 22, 2015

Since 1.12 will be released, i will remove wontfix label

@markelog markelog removed the wontfix label Dec 22, 2015

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Dec 22, 2015

Member

Last time I tried it it caused problems for IE<8. I'd rather it stayed wontfix unless we can verify IE 6-7 don't have problems with it.

Member

mgol commented Dec 22, 2015

Last time I tried it it caused problems for IE<8. I'd rather it stayed wontfix unless we can verify IE 6-7 don't have problems with it.

@mgol mgol added the wontfix label Jan 27, 2016

@cssmagic cssmagic referenced this issue in cssmagic/ChangeLog May 18, 2016

Open

jQuery #5

@jquery jquery locked as resolved and limited conversation to collaborators Jun 18, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.