Customizer cropped images issue with flex-width=false and flex-height=false

[WebTrooper]

The issue occurs in both, add_theme_support('custom-header') and WP_Customize_Cropped_Image_Control

When flex-width and flex-height are both set to true, I am still able to drag the vertical and horizontal cropping handles independently, thus changing the proportion of the cropped area of the image. The resulting "cropped" image is then stretched or squished to the correct crop size.

With flex-width and flex-height both set to false (or left to default), only the corners of the cropping area should be draggable, thus maintaining the aspect ratio. I believe it used to work that way, but now it doesn't.

Tested on WordPress 4.9, PHP 7

[WebTrooper]

Edit: Sorry, I mean when flex-width and flex-height are both set to "false".

[westonruter]

@WebTrooper is this a new issue in 4.9 or did it exist in previous versions of WordPress? Would you please check in 4.8.3?

[WebTrooper]

Tested on 4.8.3 and it works properly. It appears this is a bug that came with 4.9.

[westonruter]

@joemcgill Could this be related to #40211 or #21819?

[joemcgill]

Replying to westonruter:

@joemcgill Could this be related to #40211 or #21819?

#40211 looks like the more likely candidate. cc: @adamsilverstein.

[adamsilverstein]

I'll take a look.

[adamsilverstein]

@WebTrooper can you provide steps to reproduce with one of the default WordPress themes or some code with a control that doesn't crop as you expect it? I tried to reproduce and am seeing correct cropping behavior in the customizer with twentyseventeen and its icon and header selectors. The cropping seems unchanged from previous versions.

[WebTrooper]

Twenty Seventeen Child Theme

[WebTrooper]

Cropping the image, dragged to right handle toward the left.

[WebTrooper]

The result seen in the customizer.

[WebTrooper]

After publishing and closing the customizer.

[WebTrooper]

The actual "cropped" image.

[WebTrooper]

Sorry for the late reply... holiday weekend. I'm not sure how much code I can provide as it's a simple matter to set the args to false. Attached is a twentyseventeen child theme with code as I described (see the functions.php)

In image 1 I dragged the right handle toward the left to get a tall, narrow crop, which I shouldn't be able to do.

Image 2 shows the cropped (squished) image in the customizer.

Image 3 shows the image in the theme after publishing and closing the customizer.

Image 4 is the actual image that I right clicked and saved (not a screen shot) from my browser.

[WebTrooper]

Just to be clear, you can add the following to twentyseventeen or a childtheme thereof, to the functions.php.

function bc_twentyseventeen_custom_header_setup() {

        remove_theme_support( 'custom-header');

        add_theme_support( 'custom-header', apply_filters( 'twentyseventeen_custom_header_args', array(
                'default-image'      => get_parent_theme_file_uri( '/assets/images/header.jpg' ),
                'width'              => 2000,
                'height'             => 1200,
                'flex-height'        => false,
                'flex-width'         => false,
                'video'              => true,
                'wp-head-callback'   => 'twentyseventeen_header_style',
        ) ) );

}
add_action( 'after_setup_theme', 'bc_twentyseventeen_custom_header_setup', 11 );

[adamsilverstein]

@WebTrooper thanks for clarifying and the code to reproduce, I'll take a look at this issue again.

[adamsilverstein]

@WebTrooper Can you please give 42646.diff​ a test and let me know if this resolves the issue you were experiencing? it does in my case. This patch fixes an issue where the cropper settings were essentially overwritten after a change to make the shift key restrict dimensions to a 1:1 ration. Now they are stored and used if available, also restored after the shift drag behavior.

Thanks.

[WebTrooper]

Much as I'd like to be helpful I'm afraid I'll have to bow out of this issue. The fix does not work for me but actually, I'm not forcing crop dimensions on any of my sites anyway. It's just something I noticed while messing around and thought the dev team should know about. If I'm the only person reporting it and doesn't seem to be an issue with anybody else, then maybe it's just me. I'm not a developer and don't have a developer setup for testing and debugging, and /wp-includes/js/media/views/cropper.js doesn't seem to exist in version 4.8 or 4.9. I did the edits in /wp-includes/js/media-views.js but it didn't fix the issue. I'm sorry to have taken up your valuable time with what appears to be a non-issue. You guys do awesome work and I sincerely appreciate it. Thank you for looking into this matter. If nobody else is reporting this issue then maybe it's time to move on to more important bug fixes.

[adamsilverstein]

@WebTrooper actually, i was able to reproduce the issue you raised, and verify the commit that broke the behavior. This patch should fix the issue, hopefully we can get some additional testers to confirm.

[WebTrooper]

Okay, that's different. I didn't realize you reproduced the issue. I'll give it another shot soon as I can but probably won't be until the weekend. I'll also read up on testing, tools, protocols, etc. Ya never know, I might learn something.

[adamsilverstein]

Yes, thank you for the bug report. I used git to take wordpress back to the this commit: r41557. Before this commit, the behavior was different.

[westonruter]

@WebTrooper please test the changes in 42646.diff​ and report back if it fixes the issue you observed. Thanks!

[dd32]

Bumping to 4.9.3 due to 4.9.2s release

[WebTrooper]

Please tell me if I'm doing this wrong.

There is no /wp-includes/js/media/views/cropper.js file. In fact, there is no /wp-includes/js/media/ directory. However, the code and changes in 42646.diff appear to be identical to that of /wp-includes/js/media-views.js, so that's where I made the changes.

I commented out everything that is highlighted in red. I added everything that is highlighted in green.

There is no difference -- I can still drag the crop handles "almost" anyway I please. The only exception is that I cannot drag the bottom crop handle downward to make a larger cropped area, which was the case before editing /wp-includes/js/media-views.js

This is on a basic WP 4.9.2 install with Wordpress Beta Tester installed, set to Point Release, then ran Update.

[adamsilverstein]

@WebTrooper thanks for testing. the folder wp-includes/js/media is part of the development repo and you won't see this in a regular WordPress install. All you should need to do for this ticket is patch the media-views.js file which is sounds like you did, however perhaps you missed some part.

I'm going to capture some before/after screencasts showing the change and then get this fix committed, once its in trunk you should be able to test it more easily with the beta testers plugin.

[WebTrooper]

Adam, thank you for the explanation. Perhaps I did miss something when applying the changes. I'll try again and let you know the results. Also thank you for taking over this ticket, as though you don't already have enough on your plate :) I look forward to your screen cast. Cheers!

[WebTrooper]

The following is my exact changes to /wp-includes/js/media-views.js
I can still crop any width/height as I please. The only thing I can't do is drag the bottom crop handle downward, which I could not do before the the changes. I can still move the handles freely and crop without respect to aspect ratios. Here's a video - ​https://youtu.be/XBNMmlf1X1k

		imgOptions = _.extend(imgOptions, {
			parent: this.$el,
			onInit: function() {
				// this.parent.children().on( 'mousedown touchstart', function( e ){

				// 	if ( e.shiftKey ) {
           // Store the set ratio.
        var setRatio = imgSelect.getOptions().aspectRatio;

        // On mousedown, If no ratio is set and the shift key is down, used a 1:1 ratio.
        this.parent.children().on( 'mousedown touchstart', function( e ) {

                // If no ratio is set and the shift key is down, used a 1:1 ratio.
                if ( ! setRatio && e.shiftKey ) {
 						imgSelect.setOptions( {
							aspectRatio: '1:1'
						} );
					// } else {
					// 	imgSelect.setOptions( {
					// 		aspectRatio: false
					// 	} );
					}
				} );

           this.parent.children().on( 'mouseup touchend', function( e ) {

                   // Restore the set ratio.
                   imgSelect.setOptions( {
                           aspectRatio: setRatio ? setRatio : false
                   } );
           } );
			}
		} );
		this.trigger('image-loaded');

And here is my theme settings

function bc_twentyseventeen_custom_header_setup() {

	remove_theme_support( 'custom-header');

	add_theme_support( 'custom-header', apply_filters( 'twentyseventeen_custom_header_args', array(
		'default-image'      => get_parent_theme_file_uri( '/assets/images/header.jpg' ),
		'width'              => 2000,
		'height'             => 1200,
		'flex-height'        => false,
		'flex-width'         => false,
		'video'              => true,
		'wp-head-callback'   => 'twentyseventeen_header_style',
	) ) );

}
add_action( 'after_setup_theme', 'bc_twentyseventeen_custom_header_setup', 11 );

[westonruter]

Replying to WebTrooper:

Adam, thank you for the explanation. Perhaps I did miss something when applying the changes. I'll try again and let you know the results. Also thank you for taking over this ticket, as though you don't already have enough on your plate :) I look forward to your screen cast. Cheers!

@WebTrooper Have you added define('SCRIPT_DEBUG', true); to your wp-config.php? Without that, WP will use the minified JS instead of the JS you patch.

[WebTrooper]

Thanks Weston, that did the trick. It works perfectly now. :)

[adamsilverstein]

Perfect, thanks for confirming @WebTrooper and thanks for the tip @westonruter!

[SergeyBiryukov]

In 42595:

Media: Store and reuse image cropper ratio settings if available, instead of overwriting.

Props adamsilverstein.
Fixes #42646.

[SergeyBiryukov]

In 42596:

Media: Store and reuse image cropper ratio settings if available, instead of overwriting.

Props adamsilverstein.
Merges [42595] to the 4.9 branch.
Fixes #42646.

[SergeyBiryukov]

In 42599:

Media: Fix JSHint error after [42595].

See #42646.

[SergeyBiryukov]

In 42600:

Media: Fix JSHint error after [42595].

Merges [42599] to the 4.9 branch.
See #42646.