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

[WIP] JSoup upgrade #584

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[WIP] JSoup upgrade #584

wants to merge 4 commits into from

Conversation

johan12345
Copy link
Collaborator

@TacoTheDank already tried to upgrade JSoup in #581, but this lead to some test failures in libopac. We need to investigate these and also do some testing of search in different OPAC systems to make sure that we don't introduce bugs with the JSoup upgrade.

@johan12345 johan12345 changed the title [WIP] Jsoup upgrade [WIP] JSoup upgrade Jul 13, 2020
different handling of .text() (missing  )
@johan12345
Copy link
Collaborator Author

johan12345 commented Jul 13, 2020

Since version 1.11.1, Jsoup replaces " " (" ") characters with spaces. This was the cause of two of the test failures, and will probably also affect some implementations in the other APIs. We should further investigate this.

different handling of .text() (missing  )
@johan12345
Copy link
Collaborator Author

The test failures should be fixed now, but more testing of the parts of libopac not covered by the automated tests is required (see above).

@johan12345
Copy link
Collaborator Author

@raphaelm You updated JSoup in 11833f3, have you tested if this doesn't break anything? Especially the   replacement mentioned above may be important for some of the APIs.

At least the other changes in this PR probably need to be merged to fix the tests.

@johan12345
Copy link
Collaborator Author

see also #639 for other things broken by the dependency upgrade

@raphaelm
Copy link
Member

ouch, sorry, somehow missed this. I think I needed to upgrade to replace the (no longer really working) stetho with flipper for debugging. I'll look into this and downgrade or fix, just need to find the time.

@johan12345
Copy link
Collaborator Author

Ah, okay. Regarding Stetho issues: I think a recent Chrome update broke it, also noticed that in https://github.com/johan12345/EVMap. It still works in other Chromium-based browsers though (Microsoft Edge, Brave). These the corresponding Stetho and Chromium issues: facebook/stetho#696, https://bugs.chromium.org/p/chromium/issues/detail?id=1187142

@raphaelm
Copy link
Member

I reverted for now

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

Successfully merging this pull request may close these issues.

2 participants