Wp-rocket: 3.8 - Divi/Extra: logo is distorted when adding missing image dimensions

[vmanthos]

Before submitting an issue please check that you’ve completed the following steps:

• Made sure you’re on the latest version ✅
• Used the search feature to ensure that the bug hasn’t been reported before ✅

Describe the bug

When using:

1. The Divi theme.
2. The Add missing image dimensions feature is enabled.

the logo is distorted:

Normally, it should be like this:

Divi stores the logo's height in a custom attribute: data-height-percentage and uses the following function to calculate the logo's dimensions:

function E(e) {
            var n, r = t("body"), i = t("#logo"), o = parseInt(i.attr("data-actual-width")), a = parseInt(i.attr("data-actual-height")), s = parseInt(i.attr("data-height-percentage")), c = t("#et-top-navigation"), u = parseInt(c.attr("data-height")), l = parseInt(c.attr("data-fixed-height")), d = t("#main-header"), f = r.hasClass("et_header_style_split"), p = d.hasClass("et-fixed-header"), _ = r.hasClass("et_hide_primary_logo"), h = r.hasClass("et_hide_fixed_logo"), v = p ? u : l;
            e = void 0 !== e && e,
            f && !window.et_is_vertical_nav && (e && (v = u),
            n = o * ((v * (s / 100) + 22) / a),
            _ && (p || e) && (n = 0),
            !h || p || e || (n = 0),
            t(".et_header_style_split .centered-inline-logo-wrap").css({
                width: n
            }))
        }

File: Divi/js/custom.unified.js

Setting a fixed width/height causes the logo to be distorted.

To Reproduce
Steps to reproduce the behavior:

1. Go to MEGA/Divi.
2. Enable the Add missing image dimensions feature.
3. Visit the front-end and scroll to see the distorted logo.

Expected behavior

The logo should not be distorted.

Additional context

Related ticket: https://secure.helpscout.net/conversation/1370025086/223865/

Slack Convo: https://wp-media.slack.com/archives/C43T1AYMQ/p1608486013007400?thread_ts=1608459676.004200&cid=C43T1AYMQ

Maybe we can add a filter in the following method, so we can exclude custom attributes like data-height-percentage:
https://github.com/wp-media/wp-rocket/blob/0c6c60c8246907979bb2075e37c412f5efaeea21/inc/Engine/Media/ImageDimensions/ImageDimensions.php#L315-L329

Backlog Grooming (for WP Media dev team use only)

• [ ] Reproduce the problem
• [ ] Identify the root cause
• [ ] Scope a solution
• [ ] Estimate the effort

[arunbasillal]

So the dreaded day is here. I can already hear @webtrainingwheels say with a grin, I told you so (https://github.com/wp-media/wp-rocket/issues/3301#issuecomment-731980612)

I see few Divi related tickets, so we might have to do something here. Dev team, please advise.

[iCaspar]

We do have some specific handling for Divi around other features.
From a dev perspective, adding exceptions on a case by case basis like this is less than ideal, so if it turns out to be a wider issue, we may want to introduce a filter rather than try to keep all the exceptions catalogued. @webtrainingwheels @GeekPress @arunbasillal can we agree on an approach from a product/support perspective and we'll see if we can get it into 3.8.2 now scheduled for 1st week of January.

[arunbasillal]

@iCaspar I am not clear on the proposed solution. Do you mean adding a filter and letting customers handle it?

Or you mean adding a filter and then maintaining a list like we do for inline JS and other exclusions?

[iCaspar]

I'd prefer not to maintain a list. Because maintenance is expensive.
Option 1: We could add a filter that would enable customers to handle it. We could use that filter to disable automatically for Divi, since we already have Divi as a 3rd party case for other things.
Option 2: We could just check for Divi and bypass without a filter, as a 1-off.

[arunbasillal]

Thanks for the info @iCaspar

Option 1: We could add a filter that would enable customers to handle it. We could use that filter to disable automatically for Divi, since we already have Divi as a 3rd party case for other things.

By disable you mean disable the entire option? Or just the logo?

The ideal solution in my eyes would be:

• Have a filter that excludes certain images. Similar to what we do for LazyLoad .
• Use it for Divi in the compatibility file since we already have it.
• Not maintain a list.

This way only the logo image is affected. And if we have cases like this in the future, we can fix them with the same filter and a helper plugin.

[engahmeds3ed]

We already have this filter

https://github.com/wp-media/wp-rocket/blob/7b7f15156541bc99dbeec0770ae143cda146db49/inc/Engine/Media/ImageDimensions/ImageDimensions.php#L119

we pass here array of full img tags so we can only adjust the compatibility file to ignore this specific image or any image has this attribute.

[GeekPress]

If we can avoid asking our users to use a helper plugin and make automatic compatibility, it will the best solution whatever how it will be done on the dev side.

[hellofromtonya]

From a dev perspective, adding exceptions on a case by case basis like this is less than ideal, so if it turns out to be a wider issue

@iCaspar are you wondering if this particular issue extends beyond Divi? Or are you wondering if this is an edge case that only impacts some of Divi's users?

If it does impact all Divi users who have the feature turned on, then it does fit into our strategy of making 3rd parties compatible from within Rocket. We already have a filter we can hook into and we have an existing third party file for Divi compatibility. The fix can be applied within that file to encapsulate it while increasing Rocket <-> Divi compatibility.

What do you think @iCaspar @engahmeds3ed ?