rewrite nsMacShellService::SetDesktopBackground

RESOLVED FIXED in Firefox1.5

Status

()

Firefox
Shell Integration
P2
normal
RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: mano, Assigned: mano)

Tracking

({crash})

Trunk
Firefox1.5
PowerPC
Mac OS X
crash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

The current implementation of |nsMacShellService::SetDesktopBackground| has some
errors:

1) It crashes for non-URL image sources (i.e. the data: protocol.
2) We're getting the URL from the source attribute instead of poking
nsIImageLoadingContent::GetCurrentURI
3) In the case where the images base uri is different from the document uri,
we're passing the wrong referer to nsIWebBrowserPersist::saveURI
4) We might return the wrong nsresult values, in failures.

The attached patch fixes issues 2, 3 and 4 and partly fixes the first issue,
that is: we don't crash anymore, but we still don't actually support data: uris,
since i'm not sure what can we do about their filenames.
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → Firefox1.1
filed bug 300293 on supporting data: images.
Created attachment 188872 [details] [diff] [review]
patch

too bad i can't ask for more than one review flag in the Firefox
bugzilla-product.
Attachment #188872 - Flags: superreview?(mconnor)
Attachment #188872 - Flags: review?(jhpedemonte)

Updated

13 years ago
Attachment #188872 - Flags: review?(jhpedemonte) → review+
Comment on attachment 188872 [details] [diff] [review]
patch

r=me
Attachment #188872 - Flags: superreview?(mconnor) → superreview+

Updated

13 years ago
Attachment #188872 - Flags: approval1.8b4? → approval1.8b4+
   nsCOMPtr<nsIDOMHTMLImageElement> image(do_QueryInterface(aElement));
   if (!image)
     return NS_ERROR_INVALID_ARG;
 
do you still need that?
Comment on attachment 188872 [details] [diff] [review]
patch

also:
   nsCOMPtr<nsIDOM3Node> node(do_QueryInterface(aElement));
+  nsCOMPtr<nsIContent> content = do_QueryInterface(node, &rv);

why not directly qi to nsIContent?
Attachment #188872 - Flags: approval1.8b4+ → approval1.8b4?

Updated

13 years ago
Attachment #188872 - Flags: approval1.8b4? → approval1.8b4+
Created attachment 189211 [details] [diff] [review]
patch

good points, biese.
Attachment #188872 - Attachment is obsolete: true
Attachment #189211 - Flags: review?(mconnor)
Comment on attachment 189211 [details] [diff] [review]
patch

r=me on the changes, carrying over a=
Attachment #189211 - Flags: review?(mconnor)
Attachment #189211 - Flags: review+
Attachment #189211 - Flags: approval1.8b4+
Checking in src/nsMacShellService.cpp;
/cvsroot/mozilla/browser/components/shell/src/nsMacShellService.cpp,v  <-- 
nsMacShellService.cpp
new revision: 1.7; previous revision: 1.6
done
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.