Closed Bug 1118926 Opened 5 years ago Closed 4 years ago
Remove -moz-resolution media fragment
The way the -moz-resolution media fragment currently works prevents us from implementing other kinds of media fragments correctly, because it forces us to treat requests for the same URI with different fragment identifiers as different requests. It's a hack that we should remove. Once downscale-during-decode is implemented for ICO files, it will do everything that -moz-resolution did automatically and transparently.
Hmm, how does downscale-during-decode fix this? AIUI we implemented this to hint which image to extract from the ICO (since it's a multi-image container). Depending on the user's display DPI and/or context of where the icon is shown, we would want to select different icons.
When downscale-during-decode is implemented for ICO files, it will automatically select the appropriate resolution layer. I have not seen a use-case for selecting it manually.
One additional note, because I just realized it might not be clear: part of downscale-during-decode is that the same image can have multiple decoded versions simultaneously. We had the #-moz-resolution media fragment in part because that used to be impossible. So if you draw the same ICO file in two different places in the UI at two different sizes, the correct resolution layer will be decoded and displayed for each.
So if I have <img src=favicon.ico width=16 height=16> and favicon.ico has 16x16 and 32x32 icon... On a Retina display will this automagically use the 32x32 bitmap at full native resolution, and the 16x16 if you're on a normal display?
Yup, exactly! It decides based upon what size the image is actually _drawn_ at, in screen pixels. (Well, technically in layer pixels, if we want to get precise.)
Perfect, thanks for the clarification.
What is the status of this bug?
(In reply to Ehsan Akhgari (don't ask for review please) from comment #7) > What is the status of this bug? Patches to implement downscale-during-decode for the ICO decoder should arrive soon in bug 1201796. Once that's done, we'll remove #-moz-resolution in this bug. Also, CC'ing some folks based upon our discussion today on IRC.
We'll handle removing all the remnants of #-moz-resolution throughout the code in this bug, but it turns out that it made more sense to actually disable its behavior in bug 1201796. I'll needinfo Justin there. If anyone else wants to jump into bug 1201796 and give feedback based upon the try build there, I'd appreciate it.
In bug 1201796, #-moz-resolution stopped working. This patch removes the rest of the C++ code related to it, which now doesn't actually do anything. Comment 11 has a try job for this part.
Attachment #8663965 - Flags: review?(tnikkel)
Attachment #8663966 - Flags: review?(dolske)
Another 146 lines deleted in these two patches!
Attachment #8663965 - Flags: review?(tnikkel) → review+
Attachment #8663966 - Flags: review?(dolske) → review?(gijskruitbosch+bugs)
Attachment #8663966 - Flags: review?(gijskruitbosch+bugs) → review+
Thanks for the reviews! I've updated the test in part 2 mentioned in comment 16. As far as I can tell, these patches don't break anything in the UI. (Remember, -moz-resolution already doesn't do anything today!)
Attachment #8663966 - Attachment is obsolete: true
Depends on: 1211456
No longer blocks: 972134
Duplicate of this bug: 972134
You need to log in before you can comment on or make changes to this bug.