Make WordPress Core

Opened 5 years ago

Last modified 16 months ago

#48703 assigned task (blessed)

Update compressed images for a clean precommit:image output

Reported by: sergeybiryukov's profile SergeyBiryukov Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords:
Focuses: Cc:

Description (last modified by SergeyBiryukov)

Previously:

Background: #48203

grunt-contrib-imagemin was updated in [46404] and some GIF images were re-minified, however I still get a bunch of changed files after running grunt imagemin:

src/wp-admin/images/loading.gif
src/wp-admin/images/media-button-image.gif
src/wp-admin/images/media-button-video.gif
src/wp-admin/images/resize-2x.gif
src/wp-admin/images/resize-rtl-2x.gif
src/wp-admin/images/resize-rtl.gif
src/wp-admin/images/resize.gif
src/wp-admin/images/sort-2x.gif
src/wp-admin/images/spinner-2x.gif
src/wp-admin/images/spinner.gif
src/wp-admin/images/wpspin_light-2x.gif
src/wp-admin/images/wpspin_light.gif
src/wp-includes/images/smilies/icon_cry.gif
src/wp-includes/images/smilies/icon_lol.gif
src/wp-includes/images/smilies/icon_redface.gif
src/wp-includes/images/spinner-2x.gif
src/wp-includes/images/spinner.gif
src/wp-includes/images/wpspin-2x.gif
src/wp-includes/images/wpspin.gif

Per a quick check in Slack, this is the case for other committers as well, so it seems like the files should be updated.

Attachments (1)

Screen Shot 2019-11-22 at 10.22.17 AM.png (548.7 KB) - added by whyisjake 5 years ago.

Download all attachments as: .zip

Change History (30)

This ticket was mentioned in Slack in #core-committers by sergey. View the logs.


5 years ago

#2 @whyisjake
5 years ago

I was going to commit, but my list is different @SergeyBiryukov.

➜  wordpress-develop svn status
M       src/wp-admin/images/bubble_bg-2x.gif
M       src/wp-admin/images/bubble_bg.gif
M       src/wp-admin/images/date-button-2x.gif
M       src/wp-admin/images/date-button.gif
M       src/wp-admin/images/loading.gif
M       src/wp-admin/images/media-button-image.gif
M       src/wp-admin/images/media-button-music.gif
M       src/wp-admin/images/media-button-other.gif
M       src/wp-admin/images/media-button-video.gif
M       src/wp-admin/images/wpspin_light-2x.gif
M       src/wp-admin/images/wpspin_light.gif
M       src/wp-admin/images/xit-2x.gif
M       src/wp-admin/images/xit.gif
M       src/wp-includes/images/smilies/icon_arrow.gif
M       src/wp-includes/images/smilies/icon_biggrin.gif
M       src/wp-includes/images/smilies/icon_confused.gif
M       src/wp-includes/images/smilies/icon_cool.gif
M       src/wp-includes/images/smilies/icon_cry.gif
M       src/wp-includes/images/smilies/icon_eek.gif
M       src/wp-includes/images/smilies/icon_evil.gif
M       src/wp-includes/images/smilies/icon_exclaim.gif
M       src/wp-includes/images/smilies/icon_idea.gif
M       src/wp-includes/images/smilies/icon_lol.gif
M       src/wp-includes/images/smilies/icon_mad.gif
M       src/wp-includes/images/smilies/icon_mrgreen.gif
M       src/wp-includes/images/smilies/icon_neutral.gif
M       src/wp-includes/images/smilies/icon_question.gif
M       src/wp-includes/images/smilies/icon_razz.gif
M       src/wp-includes/images/smilies/icon_redface.gif
M       src/wp-includes/images/smilies/icon_rolleyes.gif
M       src/wp-includes/images/smilies/icon_sad.gif
M       src/wp-includes/images/smilies/icon_smile.gif
M       src/wp-includes/images/smilies/icon_surprised.gif
M       src/wp-includes/images/smilies/icon_twisted.gif
M       src/wp-includes/images/smilies/icon_wink.gif
M       src/wp-includes/images/wpspin-2x.gif
M       src/wp-includes/images/wpspin.gif
M       src/wp-includes/images/xit-2x.gif
M       src/wp-includes/images/xit.gif

#3 @whyisjake
5 years ago

I can't seem to get this script to settle. It wants to keep pulling a few more bytes out each time you run the script.

This seems a little suboptimal...

#4 @SergeyBiryukov
5 years ago

It gets weirder :)

  • The initial list from the ticket description is what I get after running the precommit:image task on Windows 10.
  • Despite SVN reporting binary file differences, there are actually no differences when comparing the file contents.
  • The only actual difference for me is src/wp-admin/images/loading.gif, which keeps changing size between 1373 and 1369 bytes.
  • At one point I also got differences in src/wp-includes/images/smilies/icon_redface.gif changing between 645 and 641 bytes, but it's not as consistent as loading.gif. This is still on Windows 10.
  • When running npm run grunt precommit:image on a Git checkout on Ubuntu 18.04 LTS, this is the list I get:
    src/wp-admin/images/loading.gif
    src/wp-admin/images/media-button-image.gif
    src/wp-admin/images/media-button-video.gif
    src/wp-admin/images/wpspin_light-2x.gif
    src/wp-admin/images/wpspin_light.gif
    src/wp-includes/images/smilies/icon_redface.gif
    src/wp-includes/images/wpspin-2x.gif
    src/wp-includes/images/wpspin.gif
    
    Again, no actual differences in file contents, just Git reporting binary file changes.

#5 @SergeyBiryukov
5 years ago

  • Milestone changed from 5.4 to Future Release
  • Summary changed from Update compressed images for 5.4 to Update compressed images

This is still relevant, but it's still not quite clear to me what causes SVN to report binary file differences if there are no actual differences when comparing the file contents.

Since it doesn't affect the build in a significant way, moving to a future release for further investigation.

Last edited 5 years ago by SergeyBiryukov (previous) (diff)

This ticket was mentioned in Slack in #core by sergey. View the logs.


5 years ago

#7 @SergeyBiryukov
5 years ago

  • Description modified (diff)
  • Milestone changed from Future Release to 5.5
  • Summary changed from Update compressed images to Update compressed images for 5.5

This ticket was mentioned in Slack in #core by sergey. View the logs.


5 years ago

#9 @whyisjake
4 years ago

  • Owner set to whyisjake
  • Status changed from new to accepted

#10 @whyisjake
4 years ago

I ran this again and got the exact same list of files from comment:2.

Last edited 4 years ago by SergeyBiryukov (previous) (diff)

#11 @desrosj
4 years ago

I am also seeing the changes from comment:2.

This ticket was mentioned in Slack in #core by whyisjake. View the logs.


4 years ago

#13 @whyisjake
4 years ago

I don't love how this isn't a guaranteed fix. Thinking we should close this out, and maybe look at another solution.

#14 @SergeyBiryukov
4 years ago

  • Milestone 5.5 deleted
  • Resolution set to maybelater
  • Status changed from accepted to closed
  • Summary changed from Update compressed images for 5.5 to Update compressed images for a clean precommit:image output

This ticket was mentioned in Slack in #core by sergey. View the logs.


4 years ago

This ticket was mentioned in Slack in #core by sergey. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-committers by sergey. View the logs.


3 years ago

#18 @SergeyBiryukov
3 years ago

  • Milestone set to 6.0
  • Resolution maybelater deleted
  • Status changed from closed to reopened

After running npm run grunt precommit:image on trunk or the 5.9 branch, I still get the list of GIF files from the ticket description without any real size difference, but also a couple of legitimate changes:

src/wp-admin/images/about-texture.png: 143 KB (146755 bytes) → 100 KB (102792 bytes)
src/wp-includes/images/w-logo-blue-white-bg.png: 4,02 KB (4119 bytes) → 3,99 KB (4094 bytes)

Let's commit these two for 6.0, and then see if the GIF file changes are stable enough now to commit as well.

This ticket was mentioned in Slack in #core by sergey. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by mike. View the logs.


3 years ago

#21 @peterwilsoncc
3 years ago

  • Owner changed from whyisjake to SergeyBiryukov
  • Status changed from reopened to assigned

@SergeyBiryukov Assigning this to you as you mentioned you were getting some significant enough savings for some images that you thought it warranted a commit.

#22 @peterwilsoncc
3 years ago

In 53293:

Built tools: Reduce file size of About page texture.

Reduce the file size of the file src/wp-admin/images/about-texture.png by running it through the grunt precommit hook.

Other modified files are not included in this commit as there is no change in file size.

See #48703.

This ticket was mentioned in Slack in #core by chaion07. View the logs.


3 years ago

#24 @costdev
3 years ago

  • Milestone changed from 6.0 to 6.1

With 6.0 RC1 starting soon, I'm moving this to the 6.1 milestone.

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


2 years ago

#26 @audrasjb
2 years ago

  • Milestone changed from 6.1 to Future Release

Unfortunately, this ticket didn’t get more momentum in 6.1 than in 6.0… sorry about that, but the proposed patch still needs testing, so let's move it to Future Release milestone, at least until it's properly reviewed and tested.

This ticket was mentioned in Slack in #core by sergey. View the logs.


2 years ago

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


16 months ago

#29 @azaozz
16 months ago

Related #58996. If/when the imagemin task is removed this ticket can be closed.

Note: See TracTickets for help on using tickets.