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

VERIFIED FIXED in seamonkey2.1a1

Status

SeaMonkey
OS Integration
--
blocker
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

({fixed-seamonkey2.0})

Trunk
seamonkey2.1a1
x86
Windows Server 2003
fixed-seamonkey2.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

8 years ago
{
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.
(Assignee)

Comment 2

8 years ago
Created attachment 400323 [details] [diff] [review]
(Av1) Just copy Firefox change

(Untested but trivial...)
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #400323 - Flags: review?
(Assignee)

Updated

8 years ago
Attachment #400323 - Flags: review? → review?(bugzilla)
(Assignee)

Comment 3

8 years ago
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.

Comment 5

8 years ago
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?

Updated

8 years ago
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.
(Assignee)

Comment 7

8 years ago
Created attachment 400359 [details] [diff] [review]
(Av1a) Support both m-1.9.2 and m-c
[Checkin: Comment 15]

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).

Comment 9

8 years ago
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.

Updated

8 years ago
Attachment #400359 - Flags: review?(kairo) → review?(bugzilla)

Comment 11

8 years ago
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.
(Assignee)

Comment 12

8 years ago
Per irc discussion, could either add the var or error out, on 1.9.2.
(Assignee)

Updated

8 years ago
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+
(Assignee)

Comment 15

8 years ago
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]
(Assignee)

Comment 16

8 years ago
(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
Last Resolved: 8 years ago
Flags: blocking-seamonkey2.1a1?
Resolution: --- → FIXED
(Assignee)

Comment 17

8 years ago
(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 :-/
(Assignee)

Comment 18

8 years ago
V.Fixed, per tinderbox.
Status: RESOLVED → VERIFIED

Updated

8 years ago
Keywords: fixed-seamonkey2.0
You need to log in before you can comment on or make changes to this bug.