131429 - 'Set as Wallpaper' should wait till image is fully loaded before setting as wallpaper

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect, P3)

Description

[Chris Nebergall ([e-mail])]

This may require a slow connection.  I'm connected at 33.6

Steps to reproduce
1.  Go to listed URL
2.  As the image is loading right click and select 'set as wallpaper'
3.  Partially loaded image is set as wallpaper

expected results:  Image will not be set as wallpaper till its finished loading
or 'Set as wallpaper' will be greyed out until the image is fully loaded.

I realize this problem is some what trivial, but considering that this feature
is for new users, there is no reason to make the process more complicated than
it has to be.  Also, this probably be a fairly common problem because on a slow
connection most people will decide if they like an image before its 100% loaded.

Comment 1

[Markus Långström]

Seeing this on 2002030803, Win 98. Confirming.

Comment 2

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Patch v1.0

This just disables the item for <img> images while we're still loading them. 
Image maps and <object>s will not be fixed by this, but it's a start... The
real issue is that we have no real way (AFAIK) from the DOM to tell that
something has loaded other than to set up a listener for "onload".  A
.hasLoaded property or something which would apply to more than just IMG and be
true only if the element has loaded successfully, would be nice...  but in the
meantime I guess we can stick to img.complete.	This is a good workaround
though for like 95% (or more?) of images on the web.

Comment 3

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Patch v1.1

(!A || (A && B)) == (!A || B)

Comment 4

[[e-mail]]

Attachment: My take on the patch

I've moved the completion test for this patch.

Comment 5

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 80250 [details] [diff] [review]
My take on the patch

Yes but in the meantime you've broken the context menu so that it never appears
for anything except an <img>.

Comment 6

[Christopher Aillon (sabbatical, not receiving bugmail)]

Taking

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 80155 [details] [diff] [review]
Patch v1.1

change |if (isWin)| to |if (isWin && this.onImage)| as discussed on IRC and
r=biesi

Comment 8

[[e-mail]]

Attachment: Address caillon's concerns

Comment 9

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Updated against tip

The other patch doesn't apply anymore.	Merged to tip.

Comment 10

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 81297 [details] [diff] [review]
Updated against tip

Still has r=biesi

Comment 11

[Blake Ross]

Comment on attachment 81297 [details] [diff] [review]
Updated against tip

sr=blake

Comment 12

[Peter Henderson]

Instead of
  navigator.appVersion.indexOf("Windows") != -1
can you say
  navigator.platform == "Win32"
becuase won't all Windows platforms say Win32? Or is this too trivial to change?

Comment 13

[Christopher Aillon (sabbatical, not receiving bugmail)]

Checked in on the trunk.  Going to see if this can make the branch.

Pete: navigator.appVersion was already there.  I just moved the code around a
bit.  We use both references throughout our code base, so you might consider
filing a bug calling for consistency.

Comment 14

[Christopher Aillon (sabbatical, not receiving bugmail)]

Drivers isn't interested in taking this for 1.0.  Resolving fixed.