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

Fix label printing in iOS 12 #27897

Merged
merged 1 commit into from
Oct 17, 2018
Merged

Fix label printing in iOS 12 #27897

merged 1 commit into from
Oct 17, 2018

Conversation

DanReyLop
Copy link
Contributor

On iOS 12, something changed that broke our shipping label printing. After clicking the "Print" button, a new tab opens but instead of seeing the label PDF you see this error:
screenshot 2018-10-17 at 08 42 45

Changes proposed in this Pull Request

This PR adds a time-out before invalidating the shipping label's PDF blob: URL so it works in iOS 12.

Testing instructions

  • Open Calypso with a Store-on-dotcom site on an iOS 12 device or simulator.
  • Go to an order and purchase a shipping label (reprinting an existing label also exposes this bug).
  • On master, you'd get an error when clicking the "Print" button (see screenshot).
  • On this PR, the PDF with the label should get loaded in a new tab.

Fixes this user-reported bug

@DanReyLop DanReyLop added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. Store Store Services labels Oct 17, 2018
@DanReyLop DanReyLop self-assigned this Oct 17, 2018
@DanReyLop DanReyLop requested a review from a team October 17, 2018 07:51
@matticbot
Copy link
Contributor

@ghost
Copy link

ghost commented Oct 17, 2018

That's a great PR description, thank you so much for your effort!

Generated by 🚫 dangerJS

Copy link
Contributor

@marcinbot marcinbot left a comment

Choose a reason for hiding this comment

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

Looks good and tests well :shipit:

Tested on the xcode simulator: reproduced the bug on wordpress.com and then verified it is fixed on calypso.live. Also tested on Chrome

@DanReyLop DanReyLop merged commit c70d533 into master Oct 17, 2018
@DanReyLop DanReyLop deleted the fix/label-printing-ios-12 branch October 17, 2018 13:54
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Oct 17, 2018
Sign up for free to 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.

3 participants