Closed Bug 516195 Opened 12 years ago Closed 12 years ago

Port |Bug 435296 - imagelib should support decode-on-draw| to SeaMonkey

Categories

(SeaMonkey :: OS Integration, defect)

x86
Windows Server 2003
defect
Not set
blocker

Tracking

(Not tracked)

VERIFIED FIXED
seamonkey2.1a1

People

(Reporter: sgautherie, Assigned: sgautherie)

References

()

Details

(Keywords: fixed-seamonkey2.0)

Attachments

(1 file, 1 obsolete file)

{
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1252798402.1252801034.26422.gz
WINNT 5.2 comm-central-trunk build on 2009/09/12 16:33:22

e:/builds/slave/comm-central-trunk-win32/build/suite/shell/src/nsWindowsShellService.cpp(607) : error C2039: 'CopyCurrentFrame' : is not a member of 'imgIContainer'
        e:\builds\slave\comm-central-trunk-win32\build\objdir\mozilla\dist\include\imgIContainer.h(42) : see declaration of 'imgIContainer'
}

SM needs the same "1 line" update as FF got.
Flags: blocking-seamonkey2.1a1?
should be trivial to fix - CopyCurrentFrame is now CopyFrame with a FRAME_CURRENT parameter.
Attached patch (Av1) Just copy Firefox change (obsolete) — Splinter Review
(Untested but trivial...)
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #400323 - Flags: review?
Attachment #400323 - Flags: review? → review?(bugzilla)
Bobby, is bug 435296 going to land on 1.9.2?
Or do c-c needs some kind of "#ifdef 1.9.3+"?
there is no chance that it will land on 1.9.2.
OK, so I forgot to search Bugzilla first and attached a patch to 435296...

I went with FRAME_CURRENT because that seemed to correspond better with the previous code; anyone know why Firefox has FRAME_FIRST there?
Attachment #400323 - Flags: review?(bugzilla) → review+
Comment on attachment 400323 [details] [diff] [review]
(Av1) Just copy Firefox change

Actually, yeah, use FRAME_CURRENT instead. If someone knows more about this, please comment.
Av1, with previous suggestion(s).
Attachment #400323 - Attachment is obsolete: true
Attachment #400359 - Flags: review?(kairo)
I made firefox use FRAME_FIRST. The function seems only to be called by nsWindowsShellService::SetDesktopBackground, and I figured that it was better to have more predictable behavior in the case of animated images, rather than just getting whatever frame was displaying at the moment.

We should probably use the same behavior for both firefox and seamonkey. I'm not particularly attached to either one, but if you do go with FRAME_CURRENT here file a bug on firefox to change it there too (maybe also ask some UX folks what they think).
Quoting from mozilla.dev.usability:

> I think Copy Image and Set As Wallpaper should be consistent. I also 
> think that the Principle of Least Surprise says that the image used 
> should be the image the user is currently seeing.
> 
> If they select this from a context menu, and the animation is continuing 
> under the menu, then of course it's a bit more complicated to know quite 
> what the user expects to happen :-)
> 
> Gerv
(In reply to comment #9)
> Quoting from mozilla.dev.usability:
 
> > If they select this from a context menu, and the animation is continuing 
> > under the menu, then of course it's a bit more complicated to know quite 
> > what the user expects to happen :-)
> > 
> > Gerv

This was the situation I was thinking about. Again though, I'm fine with either one.
Attachment #400359 - Flags: review?(kairo) → review?(bugzilla)
Comment on attachment 400359 [details] [diff] [review]
(Av1a) Support both m-1.9.2 and m-c
[Checkin: Comment 15]

Forwarding review to Mark, I'm not convinced if we'll ever support 1.9.2 with comm-central at all, and I'd rather not clutter our build system with something we never will need. Mark may have a better feeling if we need it at all.
Per irc discussion, could either add the var or error out, on 1.9.2.
Blocks: 516858
Comment on attachment 400359 [details] [diff] [review]
(Av1a) Support both m-1.9.2 and m-c
[Checkin: Comment 15]

Thunderbird does want the 1.9.2 var at the moment. As long as ifdefs don't get too complicated I think it is reasonable to support 1.9.2 for the time being. Once we branch we'll be able to drop the 1.9.1 stuff anyway.
Attachment #400359 - Flags: review?(bugzilla) → review+
Comment on attachment 400359 [details] [diff] [review]
(Av1a) Support both m-1.9.2 and m-c
[Checkin: Comment 15]

Giving aSM2.0=Standard8 as this doesn't affect 2.0.
Attachment #400359 - Flags: approval-seamonkey2.0+
Comment on attachment 400359 [details] [diff] [review]
(Av1a) Support both m-1.9.2 and m-c
[Checkin: Comment 15]


http://hg.mozilla.org/comm-central/rev/e5fceda01f27
Attachment #400359 - Attachment description: (Av1a) Support both m-1.9.2 and m-c → (Av1a) Support both m-1.9.2 and m-c [Checkin: Comment 15]
(In reply to comment #8)
> if you do go with FRAME_CURRENT
> here file a bug on firefox to change it there too (maybe also ask some UX folks
> what they think).

I leave this to you, as you know better than me what the Firefox situation is.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: blocking-seamonkey2.1a1?
Resolution: --- → FIXED
(In reply to comment #15)
> (From update of attachment 400359 [details] [diff] [review])
> 
> http://hg.mozilla.org/comm-central/rev/e5fceda01f27

Frank, sorry I missed to cite you:
all these "bugzilla@" address are just confusing to work with :-/
V.Fixed, per tinderbox.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.