Skip to content

Backport Safari fix for input changes - #9960

Merged
gaearon merged 1 commit into
react:15.6-devfrom
jquense:patch-1
Jun 14, 2017
Merged

Backport Safari fix for input changes#9960
gaearon merged 1 commit into
react:15.6-devfrom
jquense:patch-1

Conversation

@jquense

Copy link
Copy Markdown
Contributor

fixes#9956

@gaearon

Copy link
Copy Markdown
Collaborator

Can you verify this works on Safari?

@jquense

Copy link
Copy Markdown
ContributorAuthor

I can't right this moment, I yanked this straight from master (#6648) so presumably FB would be broken still if it didn't? :P

@gaearon

Copy link
Copy Markdown
Collaborator

That's my thinking too. 😄 Let's get in. Thanks.

@gaearon

Copy link
Copy Markdown
Collaborator

(I'll still wait for CI)

@gaearon

Copy link
Copy Markdown
Collaborator

Eh, seems like I broke CI with lint 😞

This is okay to merge:

Received return code 1 from: ./node_modules/.bin/gulp lint
Received return code 0 from: node ./scripts/prettier/index
Received return code 0 from: ./node_modules/.bin/gulp flow
Received return code 0 from: ./node_modules/.bin/grunt build
Received return code 0 from: ./scripts/circleci/test_extract_errors.sh
Received return code 0 from: ./scripts/circleci/track_stats.sh
Received return code 0 from: node ./addons/test

@blling

blling commented Mar 4, 2018

Copy link
Copy Markdown

Some brower do not support hasOwnProperty on DOM node, need check node.hasOwnProperty if it is defined first here :(

@nhunzaker

Copy link
Copy Markdown
Contributor

@blling Yikes! Do you know in which browsers this is the case?

@blling

Copy link
Copy Markdown

I found IE8 for now... I know React has dropped support of IE8~~, but if you could use Object.hasOwnProperty.call() is a better way.

@blling

blling commented Mar 6, 2018

Copy link
Copy Markdown

@nhunzaker do you think the issue of hasOwnProperty should be fix in React15.6.x?

@jquense

Copy link
Copy Markdown
ContributorAuthor

I don't think so, we are actively removing ie8 specific code. I don't think it makes sense to cover that case just here

@nhunzaker

Copy link
Copy Markdown
Contributor

Yeah; that's where I'm at too.

@blling

Copy link
Copy Markdown

o_o

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@jquense@gaearon@blling@nhunzaker@facebook-github-bot