Skip to content

fix(madara): properly parse srcset with image descriptors (w/x suffixes)#17518

Open
IMXEren wants to merge 1 commit into
keiyoushi:mainfrom
IMXEren:fix/madara-srcset
Open

fix(madara): properly parse srcset with image descriptors (w/x suffixes)#17518
IMXEren wants to merge 1 commit into
keiyoushi:mainfrom
IMXEren:fix/madara-srcset

Conversation

@IMXEren

@IMXEren IMXEren commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Refactored getSrcSetImage() to correctly handle comma-separated srcset entries with width (w) and pixel-density (x) descriptors. Previously the method used to compare URLs lexicographically, which breaks on srcset="link-34x68.jpg 34w, link-330x700.jpg 330w" and chooses link-34x68.jpg.

Adds IMAGE_DESCRIPTOR_REGEX companion constant and prefers highest-resolution images by descriptor value, falling back to lexicographical URL comparison when no descriptors are present.

Checklist:

  • Updated versionCode value in build.gradle.kts
  • Updated baseVersionCode in build.gradle.kts (if updated multisrc theme code)
  • Referenced all related issues in the PR body (e.g. "Closes #xyz")
  • Set the contentWarning configuration in build.gradle.kts appropriately
  • Have not changed source names
  • Have explicitly kept the id if a source's name or language were changed
  • Have tested the modifications by compiling and running the extension through Android Studio
  • Have removed web_hi_res_512.png when adding a new extension
  • This PR is AI-assisted, I have reviewed the changes manually and confirmed they are not slop

Refactored getSrcSetImage() to correctly handle comma-separated srcset
entries with width (w) and pixel-density (x) descriptors. Previously the
method used to compare URLs lexicographically, which breaks on
srcset="link-34x68.jpg 34w, link-330x700.jpg 330w" and chooses
link-34x68.jpg.

Adds IMAGE_DESCRIPTOR_REGEX companion constant and prefers
highest-resolution images by descriptor value, falling back to
lexicographical URL comparison when no descriptors are present.
@IMXEren

IMXEren commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Hey @vetleledaal, just checking in to see if everything looks good here or if anything is missing.

Thanks!

@vetleledaal vetleledaal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need additional consideration when making changes to Madara as it will update a lot of extensions (approx. 300).

I made something similar in the past, but it was specific to that one source (via #9815):

override fun String.getSrcSetImage(): String? {
/* Assumption: URL is absolute
* Assumption: descriptor is always in width
*/
return this
.split(",")
.mapNotNull { candidate ->
candidate
.trim()
.split(" ", limit = 2)
.takeIf { it.size == 2 }
?.let { (url, desc) ->
desc
.takeIf { it.endsWith("w") }
?.removeSuffix("w")
?.toIntOrNull()
?.let { size -> url to size }
}
}
.maxByOrNull { it.second }
?.first
}

.maxOfOrNull(String::toString)
protected open fun String.getSrcSetImage(): String? {
val images = this.split(",")
.map { it.trim().split("\\s+".toRegex()) }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the class to avoid recompilation of invariant regex.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants