Apply User-Agent header correctly to Android WebView by mtostenson · Pull Request #5822 · react/react-native

mtostenson · 2016-02-08T19:30:42Z

facebook-github-bot · 2016-02-08T19:31:28Z

By analyzing the blame information on this pull request, we identified @mtostenson, @mkonicek and @nicklockwood to be potential reviewers.

facebook-github-bot · 2016-02-08T19:59:30Z

@mtostenson updated the pull request.

ide · 2016-02-09T01:17:05Z

use case-insensitive comparison

facebook-github-bot · 2016-02-09T17:25:17Z

@mtostenson updated the pull request.

mkonicek · 2016-02-09T18:14:58Z

Can view.getSettings() return null, causing a NPE?

I just discovered that useragent is already it's own prop, I think I'll handle this case where the user agent is provided in the headers in the wrapper, and just pass it along to the user agent prop.

Cool, sounds good!

facebook-github-bot · 2016-02-12T23:05:19Z

@mtostenson updated the pull request.

mkonicek · 2016-02-21T23:59:06Z

@mtostenson Do you still want to make changes based on the comments above or is this good to go?

ghost · 2016-04-01T17:00:40Z

@mkonicek would you mind taking a look at this pull request? It's been a while since the last commit was reviewed.

mtostenson · 2016-04-01T17:16:48Z

@mkonicek I'm starting to think this change isn't necessary.
In my particular use case, I had the user-agent string being included with the rest of the headers, but I should probably just use the prop. I think I'll close this unless you think differently.

janicduplessis · 2016-04-05T06:07:49Z

I think the user agent header should either work or warn to use the user agent prop. It will avoid any confusion around it.

mkonicek · 2016-04-07T13:52:55Z

It is a indeed quite confusing the user agent header is set using a prop.

Thanks for the ping, I think this PR will reduce the confusion actually. Let's merge it.

@facebook-github-bot shipit

facebook-github-bot · 2016-04-07T13:57:53Z

Thanks for importing. If you are an FB employee go to Phabricator to review.

mkonicek · 2016-04-07T16:19:00Z

@mtostenson The patch doesn't apply anymore. Could you rebase please?

mkonicek · 2016-05-25T17:52:18Z

Can you please also add a Test plan showing how you verified the code fixes the issue? See https://github.com/facebook/react-native/blob/master/PULL_REQUEST_TEMPLATE.md

mtostenson · 2016-06-21T00:46:57Z

@mkonicek I just rebased and tested. Unfortunately, that link appears to be dead. Were you pointing me to this perhaps? https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md

mkonicek · 2016-10-26T15:41:31Z

@mtostenson You commented in the past:

I just discovered that useragent is already it's own prop, I think I'll handle this case where the user agent is provided in the headers in the wrapper, and just pass it along to the user agent prop.

Do you still want to do that? I think you mean this?

<WebView
userAgent='MyApp'
source={{uri:this.state.url}} />

Someone said the userAgent prop works on both iOS and Android:
#5821

It is documented for Android: http://facebook.github.io/react-native/docs/webview.html#useragent

So I think we can close this PR.

Sorry about the broken link, the pull request template has been moved, see this for an explanation of how to write a Test Plan: https://github.com/facebook/react-native/blob/master/.github/PULL_REQUEST_TEMPLATE.md

mkonicek · 2016-10-26T15:41:56Z

-import com.facebook.react.bridge.ReadableMap;
-import com.facebook.react.bridge.ReadableMapKeySetIterator;
-import com.facebook.react.bridge.WritableMap;
+import com.facebook.react.bridge.*;


Please no wildcard imports.

mkonicek · 2016-10-26T15:45:51Z

Sorry this took so long, I was on a different team and Github notifications fell through the cracks :(

mkonicek · 2016-10-26T15:49:16Z

I'll make a change internally that still sets the user agent based on the header too, addressing the nits on this PR.

mkonicek · 2016-10-26T16:04:52Z

Followup PR: #10563

Summary: Followup for #5822, addressing nits. **Test Plan** Travis CI (the author of #5822 tested the change). Closes#10563 Differential Revision: D4081826 fbshipit-source-id: f3a2e1996bf02f81fecea6e53fe1c522b8c85689

facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Feb 8, 2016

mtostenson force-pushed the user-agent branch from 6a448a6 to 6ca1bbfCompare February 8, 2016 19:53

ide reviewed Feb 9, 2016
View reviewed changes

ide added GH Review: needs-revision and removed GH Review: review-needed labels Feb 9, 2016

facebook-github-bot added GH Review: review-needed and removed GH Review: needs-revision labels Feb 9, 2016

mkonicek reviewed Feb 9, 2016
View reviewed changes

satya164 added the Needs Response from Author label Apr 1, 2016

mkonicek removed the Needs Response from Author label Apr 7, 2016

facebook-github-bot added GH Review: accepted and removed GH Review: review-needed labels Apr 7, 2016

mkonicek added the GH Review: needs-revision label Apr 7, 2016

mkonicek added Needs Response from Author and removed GH Review: accepted labels Apr 13, 2016

satya164 added Needs Response from Author and removed Triage - In Progress labels May 24, 2016

Michael Tostenson added 2 commits June 20, 2016 16:04

Apply User-Agent header correctly to Android Webview
fe94917

Use case insensitive string comparison
2c7ab4e

mtostenson force-pushed the user-agent branch from e6ac854 to 2c7ab4eCompare June 20, 2016 23:09

ghost added GH Review: review-needed and removed GH Review: needs-revision labels Jun 20, 2016

ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 12, 2016

ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 21, 2016

fredemmott removed the Needs Response from Author label Aug 11, 2016

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 11, 2016

ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 11, 2016

lacker assigned mkonicek Oct 25, 2016

mkonicek removed the GH Review: review-needed label Oct 26, 2016

mkonicek reviewed Oct 26, 2016
View reviewed changes

mkonicek closed this Oct 26, 2016

mkonicek mentioned this pull request Oct 26, 2016
Followup: Apply the User-Agent header correctly to Android WebView #10563
Closed

Uh oh!

Conversation

mtostenson commented Feb 8, 2016

Uh oh!

facebook-github-bot commented Feb 8, 2016

Uh oh!

facebook-github-bot commented Feb 8, 2016

Uh oh!

ideFeb 9, 2016

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Feb 9, 2016

Uh oh!

mkonicekFeb 9, 2016

Choose a reason for hiding this comment

Uh oh!

mtostensonFeb 10, 2016

Choose a reason for hiding this comment

Uh oh!

mkonicekFeb 10, 2016

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Feb 12, 2016

Uh oh!

mkonicek commented Feb 21, 2016

Uh oh!

ghost commented Apr 1, 2016

Uh oh!

mtostenson commented Apr 1, 2016

Uh oh!

janicduplessis commented Apr 5, 2016

Uh oh!

mkonicek commented Apr 7, 2016

Uh oh!

facebook-github-bot commented Apr 7, 2016

Uh oh!

mkonicek commented Apr 7, 2016

Uh oh!

mkonicek commented May 25, 2016

Uh oh!

mtostenson commented Jun 21, 2016

Uh oh!

mkonicek commented Oct 26, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mkonicekOct 26, 2016

Choose a reason for hiding this comment

Uh oh!

mkonicek commented Oct 26, 2016

Uh oh!

mkonicek commented Oct 26, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mkonicek commented Oct 26, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mkonicek commented Oct 26, 2016•
edited
Loading

mkonicek commented Oct 26, 2016•
edited
Loading