Closed Bug 836124 Opened 7 years ago Closed 7 years ago

Replace imgIContainer::GetCurrentFrameIsOpaque() with [noscript] FrameIsOpaque(aWhichFrame)


(Core :: ImageLib, defect)

Not set





(Reporter: seth, Assigned: seth)



(1 file, 2 obsolete files)

The |currentFrameIsOpaque| attribute on imgIContainer is not used by any addons in the addons MXR, nor by any JavaScript code of ours. It should be marked |noscript|. Furthermore, it is preferable that all methods which operate on the current frame should be replaced with versions which take an |aWhichFrame| argument, as this makes it much easier to ignore animation on particular instances of images. So the overall proposed change here is to go from this:

> readonly attribute boolean currentFrameIsOpaque;

To this:

> [noscript] boolean frameIsOpaque(in uint32_t aWhichFrame);
Summary: Replace imgIContainer::GetCurrentFrameIsOpaque() with [noscript] GetFrameIsOpaque(aWhichFrame) → Replace imgIContainer::GetCurrentFrameIsOpaque() with [noscript] FrameIsOpaque(aWhichFrame)
Whoops! Forgot to change the imgIContainer UUID. No need for a new try run.
Attachment #707923 - Flags: review?(joe)
Attachment #707917 - Attachment is obsolete: true
Attachment #707917 - Flags: review?(joe)
Comment on attachment 707923 [details] [diff] [review]
Replace GetCurrentFrameIsOpaque() with [noscript] FrameIsOpaque(aWhichFrame).

Review of attachment 707923 [details] [diff] [review]:

::: image/public/imgIContainer.idl
@@ +151,5 @@
> +   * behind it.
> +   *
> +   * @param aWhichFrame Frame specifier of the FRAME_* variety.
> +   */
> +  [noscript] boolean frameIsOpaque(in uint32_t aWhichFrame);

If you make this notxpcom too, it'll have a boolean return value instead of nsresult. Up to you.
Attachment #707923 - Flags: review?(joe) → review+
I'm fine with that. The only thing we lose is the ability to signal the caller that they passed an invalid FRAME_* enumeration value in, which doesn't seem that important because any code that does that is just wrong. I'll make that print an NS_WARNING instead. Thanks for the review!
Switched from [noscript] to [notxpcom]. Should be OK without another try run.
Attachment #707923 - Attachment is obsolete: true
This should be good to go. Requesting checkin.
Keywords: checkin-needed
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.