Skip to content

CB-11714: (windows) added check for encoding in savePhoto() without height/width#242

Merged
brody2consult merged 4 commits intoapache:masterfrom
DisruptiveMind:master
Oct 3, 2018
Merged

CB-11714: (windows) added check for encoding in savePhoto() without height/width#242
brody2consult merged 4 commits intoapache:masterfrom
DisruptiveMind:master

Conversation

@DisruptiveMind
Copy link
Copy Markdown
Contributor

Platforms affected

Windows 10

What does this PR do?

Adds extra content-type check when capturing photos without specifying targetWidth and/or targetHeight in options to preserve JPEG file extension.

What testing has been done on this change?

ditto

Checklist

  • Reported an issue in the JIRA database
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
  • Added automated test coverage as appropriate for this change.

@cordova-qa
Copy link
Copy Markdown

Cordova CI Build has completed successfully.

Commit - Link
Dashboard - Link

Builder Name Console Output Test Report Device Logs
Windows 8.1 Store Link Link Link
Windows 10 Store Link Link Link
Windows 8.1 Phone Link Link Link
iOS Link Link Link
Android Link Link Link

@janpio
Copy link
Copy Markdown
Member

janpio commented Sep 16, 2018

Hey @DisruptiveMind , there seems to be a merge conflict now. Could you maybe fix this? Thanks.

@janpio janpio added the bug label Sep 17, 2018
@janpio
Copy link
Copy Markdown
Member

janpio commented Oct 1, 2018

Thanks @DisruptiveMind.

Tests are now failing because of

/Users/travis/build/apache/cordova-plugin-camera/src/windows/CameraProxy.js
  790:67  error  Strings must use singlequote  quotes
✖ 1 problem (1 error, 0 warnings)
  1 error, 0 warnings potentially fixable with the `--fix` option.

This doesn't make too much sense to me, especially as line 790 contains no quotes at all.

Can you maybe rebase on master? I hope this will go away with more recent test configuration. Thanks!

@janpio
Copy link
Copy Markdown
Member

janpio commented Oct 3, 2018

Closing and re-opening to trigger a new CI/test run with new PR merge.

@janpio janpio closed this Oct 3, 2018
@janpio janpio reopened this Oct 3, 2018
@brody2consult
Copy link
Copy Markdown

Looks OK to me, merging

@brody2consult brody2consult merged commit dc73954 into apache:master Oct 3, 2018
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.

4 participants