Or, in general, Image Resize Filter breaks any attributes that have dashes in them.

To fix this, we have to update a regular expression:

    preg_match_all('/([a-z]+)[ ]*=[ ]*"([^"]*)"/i', $img_tag, $attribute_matches);

to:

    preg_match_all('/([a-z\-]+)[ ]*=[ ]*"([^"]*)"/i', $img_tag, $attribute_matches);

Patch attached.

CommentFileSizeAuthor
#7 1708366-7.patch741 byteswim leers
image_resize_filter-data_attributes.patch742 byteswim leers

Comments

quicksketch’s picture

quicksketch
he/him
commented

Thanks Wim! Maybe we should allow numbers in there too? Data attributes can contain any alphanumeric character I believe.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

I just looked it up, and the spec actually says no such thing:

A custom data attribute is an attribute in no namespace whose name starts with the string "data-", has at least one character after the hyphen, is XML-compatible, and contains no characters in the range U+0041 to U+005A (LATIN CAPITAL LETTER A to LATIN CAPITAL LETTER Z).

So AFAICT, the patch above covers all use cases :) We could make the patch more strict, not allowing dashes everywhere, but only when an attribute begins with data-. But I personally think that's not really necessary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

Based on #2 and the simplicity of the patch, tentatively setting to RTBC.

jcisio’s picture

jcisio commented
Status: Reviewed & tested by the community » Needs work

That regex does not match "data-n9".

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Needs review

#4: are you sure that numeric characters are allowed?

jcisio’s picture

jcisio commented
Status: Needs review » Needs work

Yes, if your quote #2 is correct.

After data-, any character that is XML-compatible and not A-Z is allowed. That means "data-n9" or even "data-n ͦ9" (well, we don't really need to support this case, I think) are both allowed.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new 1708366-7.patch741 bytes

#6: Hm, fair enough.

In that case, we can just use [\w\-]+ instead of [a-z\-]+ (original: [a-z]+).

Rerolled.

phoehne’s picture

phoehne commented

Stumbled across the same problem,

patch works perfect.

Thanks Peter

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

Setting to RTBC as per #8.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Bump. quicksketch?

jcisio’s picture

jcisio commented

I've just read #6 again, and it says A-Z are not allowed. I let you to decide change the issue to NW or leave it as RTBC, as we don't really have to follow specs.

  • ram4nd committed c10b56c on 7.x-1.x authored by Wim Leers 
    Issue #1708366 by Wim Leers: Breaks data- attributes
ram4nd’s picture

ram4nd commented
Assigned: wim leers » Unassigned
Issue summary: View changes
Status: Reviewed & tested by the community » Fixed
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Yay, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.