Skip to content
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.

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

(#7495) - Fix how fetch resolves absolute URIs #7496

Merged
merged 3 commits into from
Nov 23, 2018

Conversation

ptitjes
Copy link
Contributor

@ptitjes ptitjes commented Oct 17, 2018

If the provided path is an absolute path, then resolve the path on the host directly.

@ptitjes
Copy link
Contributor Author

ptitjes commented Nov 1, 2018

@daleharvey, @garrensmith can one of review my PR please ?

Resolution of #7495 is mandatory for any plugin that needs to communicate with CouchDB directly, as does PouchDB Authentication.

It would be great if it could be solved and that we have a patch release along with #7395.

@garrensmith
Copy link
Member

garrensmith commented Nov 1, 2018 via email

@daleharvey
Copy link
Member

Hey sorry for the delay, you posted just as I went on holiday, will get to this by tomorrow latest, cheers

@ptitjes
Copy link
Contributor Author

ptitjes commented Nov 5, 2018

No problem guys. I was just worried to miss the release, which I feel might come soon :)

@daleharvey daleharvey mentioned this pull request Nov 11, 2018
@daleharvey
Copy link
Member

lol so I looked into this, silly that I missed it before, we use es5 aside from modules so let needs to be var, this straight up breaks the entire build during minification which is a silly bug on our end somewhere but an easy fix

@ptitjes
Copy link
Contributor Author

ptitjes commented Nov 17, 2018

Oh sorry. That's too much switching between projects... I'll try to fix that tomorrow!

@ptitjes
Copy link
Contributor Author

ptitjes commented Nov 19, 2018

@daleharvey So there is only a failure with message Unable to get property 'rows' of undefined or null reference (http://127.0.0.1:8000/tests/integration/test.design_docs.js:90)" on both IE11:Windows 10 tests. Any hint on how it is related to my changes ?

@daleharvey
Copy link
Member

@ptitjes Its very unlikely this bug is related to your changes, ie (and iphone) are quite unstable browsers in when run in selenium, will restart to confirm and likely this is good to go when green

@daleharvey
Copy link
Member

Actually I should have looked more carefully, the failures are both ie11 and you are using startsWith, probably easiest to just switch that to a regex or can just use a replacement function (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/startsWith) but not polyfill, we dont want to touch the string prototype

@ptitjes
Copy link
Contributor Author

ptitjes commented Nov 21, 2018

@daleharvey I fixed it by using substr. There is now only one test failing with a timeout :D

@ptitjes
Copy link
Contributor Author

ptitjes commented Nov 23, 2018

Finally passing on IE :) <3

Copy link
Member

@daleharvey daleharvey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job cheers

@ptitjes
Copy link
Contributor Author

ptitjes commented Nov 23, 2018

@daleharvey Is the « don't use the big green button » still the policy ? Or we can use the "Squash and merge" or "Rebase and merge" ?

@daleharvey daleharvey merged commit d126805 into pouchdb:master Nov 23, 2018
@ptitjes ptitjes deleted the fix-fetch-absolute-uri branch November 23, 2018 23:07
@daleharvey
Copy link
Member

Did we write dont use the big green button down anywhere? github fixed it a while ago so we have set so it will squash and merge without making a merge commit

@daleharvey
Copy link
Member

ie using the big green button is fine now

@ptitjes
Copy link
Contributor Author

ptitjes commented Nov 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants