Negative values in column's width redistribution

[lukewalczak]

Describe the bug

During the sanity testing before mobile release @chipsnyder noticed that we are able to get negative percentages in Columns and mentioned about it in the comment. Since the logic for width redistribution is shared the same occurs on web editor.

To reproduce
Steps to reproduce the behavior:

1. Create a new Blog Post
2. Add A Columns block
3. Select 70/30
4. Change column 1 to 90%
5. Set Column 2 to 10% (or around there)
6. Add a third column

Result:
With those numbers I got: 73.34%, -6.66% and 33.33%.

The same will happen when adding new column to the following columns: 90/5/5 and many other.

The problem is the wrong math within the function called getRedistributedColumnWidths located in columns utils.

export function getRedistributedColumnWidths(
	blocks,
	availableWidth,
	totalBlockCount = blocks.length
) {
	const totalWidth = getTotalColumnsWidth( blocks, totalBlockCount );
	const difference = availableWidth - totalWidth;
	const adjustment = difference / blocks.length;

	return mapValues( getColumnWidths( blocks, totalBlockCount ), ( width ) =>
		toWidthPrecision( width + adjustment )
	);

Mentioned method is assuming that difference (difference between available width and new columns width sum) is divided equally between columns and subtracted from each of them. As a result we can get negative values, since adjustment has higher value than width.

Let's consider it on the example from reproduction steps:

• Two columns: 90% and 10%
• New column will have width equal 33% (100 / 3)
• availableWidth refers to the sum of first two columns after adding the third one: 66.67 (100 - 33)
• total width refer to the sum of all columns: 100
• difference is -33.33 (66.67 - 100)
• adjustment is -16.665 (-33.33 / 2)

Finally we have

• first column: 90 - 16.665
• second column: 10 - 16.665
• third column: 33.33

Possible problem solution

Imo the mechanism should be a bit different and width should be calculated proportionally to the totalWidth:

export function getRedistributedColumnWidths(
	blocks,
	availableWidth,
	totalBlockCount = blocks.length
) {
	const totalWidth = getTotalColumnsWidth( blocks, totalBlockCount );

	return mapValues( getColumnWidths( blocks, totalBlockCount ), ( width ) => {
		const newWidth = ( availableWidth * width ) / totalWidth;
		return toWidthPrecision( newWidth );
	} );
}

As a result we get positive values, divided proportionally:

• first column: 60
• second column: 6.67
• third column: 33.33

Expected behavior

Columns shouldn't have negative width values

Screenshots

Attached above.
If applicable, add screenshots to help explain your problem.

Editor version (please complete the following information):

• WordPress version: [e.g: 5.3.2]
• Does the website has Gutenberg plugin installed, or is it using the block editor that comes by default? [e.g: "gutenberg plugin", "default"]
• If the Gutenberg plugin is installed, which version is it? [e.g., 7.6]

Desktop (please complete the following information):

• OS: [e.g. iOS]
• Browser [e.g. chrome, safari]
• Version [e.g. 22]

Smartphone (please complete the following information):

• Device: [e.g. iPhone6]
• OS: [e.g. iOS8.1]
• Browser [e.g. stock browser, safari]
• Version [e.g. 22]

Additional context

• To report a security issue, please visit the WordPress HackerOne program: https://hackerone.com/wordpress.

[chipsnyder]

Hey @tellthemachines and @gziolo, I saw that you two had worked in this area somewhat recently with the PR for fixing some unit tests. I was wondering if you two had enough context here to review @lukewalczak's plan.

[gziolo]

I don’t know how getRedistributedColumnWidths should work but what you shared makes a lot of sense. It definitely seems buggy to calculate negative values. I checked unit tests and they cover only 2 columns. I’m sure adding test cases that verify use cases you identified for 3 (or even more) columns as needing changes would be a good start here.

[lukewalczak]

@gziolo thanks for your answer, I'm planning to prepare the PR with the fix along with additional test to cover edgy cases