Rename "decode-on-draw" to "decode-only-on-draw" to make its meaning more obvious

RESOLVED FIXED in Firefox 39

Status

()

Core
ImageLib
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: seth, Assigned: seth)

Tracking

unspecified
mozilla39
Points:
---

Firefox Tracking Flags

(firefox39 fixed, relnote-firefox -)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
We will *always* decode-on-draw, in the sense that if we don't have a decoded version of an image and someone calls imgIContainer::Draw, we start decoding the image. The "image.mem.decodeondraw" pref actually didn't do anything.

It recently got repurposed to mean that we would *only* decode-on-draw, and ignore calls to imgIContainer::RequestDecode and the like. However, since we will always decode-on-draw, using "decode-on-draw" to mean this is a bit confusing.

In this bug we'll rename "decode-on-draw" to "decode-only-on-draw" everywhere that it occurs. This make it much clearer what this pref means.
(Assignee)

Comment 1

4 years ago
Created attachment 8552305 [details] [diff] [review]
Rename decode-on-draw to decode-only-on-draw

Here's the patch.
Attachment #8552305 - Flags: review?(tnikkel)
Attachment #8552305 - Flags: review?(tnikkel) → review+
(Assignee)

Comment 2

3 years ago
Created attachment 8582726 [details] [diff] [review]
Rename decode-on-draw to decode-only-on-draw

Ugh, I shouldn't have waited so long to land this. Belated thanks for the
review. Here's a rebased version.
(Assignee)

Updated

3 years ago
Attachment #8552305 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/77347e90d2c7
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Release Note Request (optional, but appreciated)
[Why is this notable]: Renaming a preference, the users that have modified this preference should know about it.
[Suggested wording]: Preference image.mem.decodeondraw has been renamed to image.decode-only-on-draw.enabled
[Links (documentation, blog post, etc)]:
relnote-firefox: --- → ?
(Assignee)

Comment 6

3 years ago
FWIW the old pref literally did not do anything. This one only works on B2G right now and I anticipate removing it soon. I'm not keen on publicizing it for that reason.
Sounds like this may not need a release note, then. 
Milan, if you disagree can you needinfo me or renominate this for a note? Thanks!
relnote-firefox: ? → -
Flags: needinfo?(milan)
(In reply to Seth Fowler [:seth] from comment #6)
> FWIW the old pref literally did not do anything. This one only works on B2G
> right now and I anticipate removing it soon. I'm not keen on publicizing it
> for that reason.

It will be good, as we now probably have 2 preferences for doing the same thing:
image.decode-only-on-draw.enabled
image.decode-immediately.enabled
I'm good with whatever you guys decide.
Flags: needinfo?(milan)
(Assignee)

Comment 10

3 years ago
(In reply to Virtual_ManPL [:Virtual] from comment #8)
> (In reply to Seth Fowler [:seth] from comment #6)
> > FWIW the old pref literally did not do anything. This one only works on B2G
> > right now and I anticipate removing it soon. I'm not keen on publicizing it
> > for that reason.
> 
> It will be good, as we now probably have 2 preferences for doing the same
> thing:
> image.decode-only-on-draw.enabled
> image.decode-immediately.enabled

We don't, actually. "image.decode-immediately.enabled", just as it sounds, decodes images immediately when it's enabled. "image.decode-only-on-draw.enabled" decodes images only when something attempts to draw them. But the normal behavior isn't either of those things, it's "apply a bunch of heuristics to decide which images to decode and when". The reason I added "image.decode-immediately.enabled" was because lots of people thought that setting "image.decode-only-on-draw.enabled" to false would cause all images to be decoded immediately, but that is not true.
(Assignee)

Comment 11

3 years ago
(In reply to Liz Henry (:lizzard) from comment #7)
> Sounds like this may not need a release note, then. 
> Milan, if you disagree can you needinfo me or renominate this for a note?
> Thanks!

Let's not add a release note for this.

We could consider adding one for bug 1149893, which added "image.decode-immediately.enabled". Though I'm not super enthusiastic about that, either. That one will arrive in 39.
You need to log in before you can comment on or make changes to this bug.