253627 - [FIXr]nsImageBoxFrame and nsTreeBodyFrame should use nsContentUtils::LoadImage

Status: RESOLVED FIXED

(Core :: XUL, defect, P3)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

From bug 236889 comment 7:
> >Index: layout/xul/base/src/nsImageBoxFrame.cpp
> >+    nsCOMPtr<imgILoader> il(do_GetService("@mozilla.org/image/loader;1", &rv));
> ...etc
> 
> I think I'd rather this used nsContentUtils::LoadImage (have to hack the
> makefile to make it includable, but that's not so hard).  For that matter, I
> would sort of like it if we did a nsContentUtils::CanLoadImage check here, with
> mContent as the context.
> 
> >Index: layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp
> 
> >@@ -1661,48 +1664,53 @@ nsTreeBodyFrame::GetImage(PRInt32 aRowIn
> >+      nsCOMPtr<imgILoader> il =
> >+        do_GetService("@mozilla.org/image/loader;1", &rv);
> 
> Again, I'd prefer it if this used nsContentUtils (for example, this code screws
> up loadgroups in the call to LoadImage -- it just passes null).
> 
> If you really don't have the time to convert those LoadImage callers right now,
> file a followup bug on that, ok?

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 154792 [details] [diff] [review]
Patch

The REQUIRES change is needed because nsContentUtils.h includes nsIPref.h (and
needs typdefs from that file; jst was against forward-declaring and copying the
typedefs).

The nsContentUtils changes are just minor cleanup to make it include less
stuff.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 4

[hub-san]

Folks, it seemed that the patch which fixed this bug caused bug 256172.  See bug
256172 comment 2.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This landed on Aviary, although no-one mentioned it in the bug here. And I don't
see any reason why this needed to land there.

It needs to land on 1.7 too. And then the regression fix in bug 256172 needs to
land... sigh.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Correction; it did not land on Aviary. I guess I was looking at the wrong LXR
branch. I went and looked again because I had a nagging feeling that bz would
never have landed this on Aviary :-)