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 base64 encoding when using chars from different range #5022

Merged
merged 1 commit into from
Oct 27, 2020

Conversation

swissspidy
Copy link
Collaborator

Summary

Quick follow-up to #4859 / #4805 that fixes issues with TextDecoder, which did not work as expected.

Relevant Technical Choices

  • Using String.fromCharCode again as originally envisioned, but now with a twist to avoid passing too many arguments.

To-do

N/A

User-facing changes

N/A

Testing Instructions

  1. Ensure WEBSTORIES_DEV_MODE is false
  2. Write a new story with title Hello 🌍 - これはサンプルです。
  3. Verfiy that saving/publishing works as expected

Fixes #

@swissspidy swissspidy added Type: Bug Something isn't working Group: WordPress Changes related to WordPress or Gutenberg integration Pod: WP & Infra labels Oct 27, 2020
@google-cla google-cla bot added the cla: yes label Oct 27, 2020
@github-actions
Copy link
Contributor

Size Change: -10 B (0%)

Total Size: 1.4 MB

ℹ️ View Unchanged
Filename Size Change
assets/css/edit-story.css 909 B 0 B
assets/css/stories-dashboard.css 939 B 0 B
assets/css/web-stories-embed-block.css 515 B 0 B
assets/js/chunk-fonts-********************.js 43.7 kB 0 B
assets/js/chunk-web-stories-template-0-********************.js 10.2 kB 0 B
assets/js/chunk-web-stories-template-1-********************.js 10.2 kB 0 B
assets/js/chunk-web-stories-template-2-********************.js 9.97 kB 0 B
assets/js/chunk-web-stories-template-3-********************.js 10.5 kB 0 B
assets/js/chunk-web-stories-template-4-********************.js 10.6 kB 0 B
assets/js/chunk-web-stories-template-5-********************.js 6.83 kB 0 B
assets/js/chunk-web-stories-template-6-********************.js 9.87 kB 0 B
assets/js/chunk-web-stories-template-7-********************.js 9.75 kB 0 B
assets/js/chunk-web-stories-textset-0-********************.js 5.25 kB 0 B
assets/js/chunk-web-stories-textset-1-********************.js 6.58 kB 0 B
assets/js/chunk-web-stories-textset-2-********************.js 7.83 kB 0 B
assets/js/chunk-web-stories-textset-3-********************.js 15.2 kB 0 B
assets/js/chunk-web-stories-textset-4-********************.js 4.38 kB 0 B
assets/js/chunk-web-stories-textset-5-********************.js 5.68 kB 0 B
assets/js/chunk-web-stories-textset-6-********************.js 5.47 kB 0 B
assets/js/chunk-web-stories-textset-7-********************.js 10.3 kB 0 B
assets/js/edit-story.js 529 kB -7 B (0%)
assets/js/stories-dashboard.js 593 kB -3 B (0%)
assets/js/web-stories-activation-notice.js 74.1 kB 0 B
assets/js/web-stories-embed-block.js 17.5 kB 0 B

compressed-size-action

@swissspidy swissspidy added the P0 Critical, drop everything label Oct 27, 2020
Copy link
Contributor

@spacedmonkey spacedmonkey 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. Do you want to me test this?

@swissspidy
Copy link
Collaborator Author

If you've got a spare minute :-)

The tests are much more reliable now though, as nothing is mocked anymore.

@codecov
Copy link

codecov bot commented Oct 27, 2020

Codecov Report

Merging #5022 into main will decrease coverage by 0.43%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5022      +/-   ##
==========================================
- Coverage   76.43%   76.00%   -0.44%     
==========================================
  Files         921      921              
  Lines       16308    16309       +1     
==========================================
- Hits        12465    12395      -70     
- Misses       3843     3914      +71     
Flag Coverage Δ
#karmatests 53.78% <0.00%> (+2.20%) ⬆️
#unittests 65.76% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
assets/src/edit-story/app/api/base64Encode.js 100.00% <100.00%> (ø)
assets/src/edit-story/elements/image/icon.js 0.00% <0.00%> (-100.00%) ⬇️
...sets/src/edit-story/elements/media/visibleImage.js 20.00% <0.00%> (-80.00%) ⬇️
assets/src/edit-story/utils/getMediaBaseColor.js 10.34% <0.00%> (-79.32%) ⬇️
...ets/src/edit-story/app/media/utils/preloadImage.js 11.11% <0.00%> (-77.78%) ⬇️
assets/src/edit-story/utils/useDoubleClick.js 8.00% <0.00%> (-72.00%) ⬇️
assets/src/edit-story/elements/media/frame.js 28.57% <0.00%> (-71.43%) ⬇️
assets/src/edit-story/elements/image/layer.js 33.33% <0.00%> (-66.67%) ⬇️
...story/components/panels/backgroundOverlay/panel.js 11.11% <0.00%> (-66.67%) ⬇️
...mponents/canvas/singleSelectionMoveable/useDrag.js 36.84% <0.00%> (-52.64%) ⬇️
... and 39 more

@swissspidy swissspidy merged commit 7c214c7 into main Oct 27, 2020
@swissspidy swissspidy deleted the fix/base64 branch October 27, 2020 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Group: WordPress Changes related to WordPress or Gutenberg integration P0 Critical, drop everything Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants