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.

Sign up for GitHub

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

Jump to bottom

Fixing image downloading not working in some cases #291

Merged
merged 1 commit into from
Mar 31, 2023
Merged

Fixing image downloading not working in some cases #291

merged 1 commit into from
Mar 31, 2023

Conversation

matiasbenedetto
Copy link
Contributor

What ?

Fixing image downloading and bundling into the theme not working in some cases.

How ?

Instead of PHP function file_get_contents() we are using the WordPress function download_url() that seems to be working in most scenarios.

Why ?

Some users reported this functionality not working in their environments. See #280 as example.

How to test:

  1. Add an image to a template or template part
  2. Export / Overwrite the theme
  3. See if the image on the assets folder downloaded correctly (not 0-byte size).

Fixes: #280

@matiasbenedetto
Copy link
Contributor Author

This zip file contains the plugin version in case you want to download it and test it in your environment:

create-block-theme.zip

@matiasbenedetto matiasbenedetto mentioned this pull request Mar 27, 2023
Closed
mikachan
mikachan reviewed Mar 29, 2023
View reviewed changes
Copy link
Member

@mikachan mikachan left a comment

Choose a reason for hiding this comment

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

I'm struggling to reproduce the original issue in my testing, so I'm unable to confirm if this fixes the issue. However, I can confirm that this PR works just the same for me, and I see my included images.

It would be awesome if @decrecementofeliz has time to test this, or someone else who has also seen this issue before.

I was also thinking that perhaps we should add improved error handling around the asset management, to handle potential silent errors such as those caused by server settings. e.g. should the plugin check for PHP settings such as upload_max_filesize before running add_media_to_local ? (Not for this PR though!)

@matiasbenedetto
Copy link
Contributor Author

I was also thinking that perhaps we should add improved error handling around the asset management, to handle potential silent errors such as those caused by server settings. e.g. should the plugin check for PHP settings such as upload_max_filesize before running add_media_to_local ? (Not for this PR though!)

Yes! that would be very useful.

@mikachan mikachan mentioned this pull request Mar 31, 2023
Open
mikachan
mikachan approved these changes Mar 31, 2023
View reviewed changes
Copy link
Member

@mikachan mikachan left a comment

Choose a reason for hiding this comment

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

Approving based on the testing by @decrecementofeliz here. Thank you!

@matiasbenedetto
Copy link
Contributor Author

Thanks @mikachan @decrecementofeliz !

@matiasbenedetto matiasbenedetto merged commit b5a0a0a into trunk Mar 31, 2023
@matiasbenedetto matiasbenedetto deleted the fix/280 branch March 31, 2023 16:05
mikachan added a commit that referenced this pull request Apr 5, 2023
@mikachan
Squashed merge commit with trunk
a4397de 
commit 0374c69
Merge: 83de623 072da09
Author: Sarah Norris <[email protected]>
Date:   Tue Apr 4 16:15:50 2023 +0100

    Merge pull request #297 from WordPress/add/font-family-labels

    Add labels around Google font family checkbox controls

commit 83de623
Merge: 58491a7 b814f29
Author: Sarah Norris <[email protected]>
Date:   Tue Apr 4 16:13:48 2023 +0100

    Merge pull request #296 from WordPress/try/add-current-wp-version

    Add current WordPress version to style.css and readme.txt

commit 58491a7
Author: github-actions <[email protected]>
Date:   Mon Apr 3 20:54:08 2023 +0000

    Version bump & changelog update

commit 3a58a30
Merge: bb36a3a fd779d3
Author: Jeff Ong <[email protected]>
Date:   Mon Apr 3 11:27:38 2023 -0400

    Merge pull request #292 from WordPress/try/site-editor-overwrite

    Add Export (Clone) to site editor

commit fd779d3
Author: Jeff Ong <[email protected]>
Date:   Mon Apr 3 11:26:11 2023 -0400

    Carry forward previous theme tags.

commit 072da09
Author: Sarah Norris <[email protected]>
Date:   Sat Apr 1 21:39:54 2023 +0100

    Add checkbox labels to font family table header

commit 54318c6
Author: Sarah Norris <[email protected]>
Date:   Sat Apr 1 21:39:22 2023 +0100

    Add checkbox labels to font family variants

commit b814f29
Author: Sarah Norris <[email protected]>
Date:   Sat Apr 1 20:54:04 2023 +0100

    Add current WP version to readme.txt

commit f4070b2
Author: Sarah Norris <[email protected]>
Date:   Sat Apr 1 20:53:50 2023 +0100

    Add current WP version to style.css

commit bb36a3a
Merge: b5a0a0a 8c4098a
Author: Matias Benedetto <[email protected]>
Date:   Fri Mar 31 17:35:57 2023 -0300

    Merge pull request #293 from WordPress/update/google-fonts-json-_4537839996

    Update Google Fonts JSON data from API

commit cd5f862
Author: Matias Benedetto <[email protected]>
Date:   Fri Mar 31 17:22:36 2023 -0300

    setting the filename of the exported zip file

commit b5a0a0a
Merge: e3f9193 260d115
Author: Matias Benedetto <[email protected]>
Date:   Fri Mar 31 13:05:52 2023 -0300

    Merge pull request #291 from WordPress/fix/280

    Fixing image downloading not working in some cases

commit 1b19d29
Author: Jeff Ong <[email protected]>
Date:   Thu Mar 30 12:24:35 2023 -0400

    Make linter happy.

commit b86a1f1
Author: Jeff Ong <[email protected]>
Date:   Thu Mar 30 12:23:42 2023 -0400

    Rename file.

commit 6efb440
Author: Jeff Ong <[email protected]>
Date:   Wed Mar 29 17:27:42 2023 -0400

    Add validation to form button, improve spacing.

commit b5c6a17
Author: Jeff Ong <[email protected]>
Date:   Wed Mar 29 17:11:01 2023 -0400

    Async/await, update icon, button.

commit 8bc38ef
Author: Jeff Ong <[email protected]>
Date:   Wed Mar 29 16:40:00 2023 -0400

    Use tt3 as placeholder

    Co-authored-by: Ben Dwyer <[email protected]>

commit 16f3c5c
Author: Matias Benedetto <[email protected]>
Date:   Wed Mar 29 15:50:17 2023 -0300

    Implementing WordPress apiFetch replacing the browser's fetch function

commit 4870efb
Author: Jeff Ong <[email protected]>
Date:   Mon Mar 27 17:47:44 2023 -0400

    Lint php.

commit ebdf773
Author: Jeff Ong <[email protected]>
Date:   Mon Mar 27 17:47:22 2023 -0400

    Site editor rest API export.

commit a7b2de4
Author: Jeff Ong <[email protected]>
Date:   Mon Mar 27 13:49:35 2023 -0400

    Add panel to sidebar in site editor.

commit 8c4098a
Author: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Date:   Tue Mar 28 00:15:24 2023 +0000

    Updating file

commit 260d115
Author: Matias Benedetto <[email protected]>
Date:   Mon Mar 27 15:48:44 2023 -0300

    fixing image downloading not working in some cases
@carolinan
Copy link

Is this part of the released plugin? or do I need to use the development version for images to be added to the assets?

@carolinan
Copy link

OK, I found the changelog, but I am still experiencing issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikachan mikachan mikachan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Exported assets/images with 0 bytes
3 participants
@matiasbenedetto @carolinan @mikachan