Closed Bug 233890 Opened 21 years ago Closed 20 years ago

some cleanup in a few layout files

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.7beta

People

(Reporter: Biesinger, Assigned: Biesinger)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
Attachment #141169 - Flags: superreview?(bzbarsky)
Attachment #141169 - Flags: review?(bzbarsky)
Comment on attachment 141169 [details] [diff] [review]
patch

>Index: content/shared/src/nsImageMapUtils.cpp
>+        nsCOMPtr<nsIDOMHTMLMapElement> map(do_QueryInterface(element));
>+        return map;

This makes me a bit queasy (as there has been some talk of putting some of the
ODM interfaces in tearoffs and that would make this code very very unhappy.

I'll try to read the rest sometime...
ok, I can use CallQueryInterface and return an already_AddRefed pointer if you want
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.7beta
That would be better.
Attached patch patch v2Splinter Review
now uses already_AddRefed.

this also contains a few additional changes (nsImageLoader and nsPresShell)
Attachment #141169 - Attachment is obsolete: true
Attachment #141254 - Flags: superreview?(bzbarsky)
Attachment #141254 - Flags: review?(bzbarsky)
Attachment #141169 - Flags: superreview?(bzbarsky)
Attachment #141169 - Flags: review?(bzbarsky)
Comment on attachment 141254 [details] [diff] [review]
patch v2

>Index: layout/xul/base/src/nsImageBoxFrame.cpp
>+nsImageBoxFrame::GetLoadGroup()
>+  return doc->GetDocumentLoadGroup().get(); // already_AddRefed

No need for the .get() there.

>Index: layout/xul/base/src/nsImageBoxFrame.h

>-  nsCOMPtr<nsIURI> mURI; // The URI of the image.
>+  nsCOMPtr<nsIURI> mURI; ///< The URI of the image.

What's the reason for this change?

>-  PRPackedBool mUseSrcAttr; // Whether or not the image src comes from an attribute.
>+  PRPackedBool mUseSrcAttr; ///< Whether or not the image src comes from an attribute.
>-  nsRect mSubRect; // If set, indicates that only the portion of the image specified by the rect should be used.
>+  nsRect mSubRect; ///< If set, indicates that only the portion of the image specified by the rect should be used.

Same.

r+sr=bzbarsky with that issue addressed.
Attachment #141254 - Flags: superreview?(bzbarsky)
Attachment #141254 - Flags: superreview+
Attachment #141254 - Flags: review?(bzbarsky)
Attachment #141254 - Flags: review+
doxygen can generate documentation for ///< comments. The < indicates that it
referes to the thing declared in front of the comment. I believe /**< */ would
also work.

see http://www.stack.nl/~dimitri/doxygen/docblocks.html#memberdoc

I'll make the other change.
checked in
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
It looks like this may have slowed down pageload a little, although it's hard to
tell.
hm... luna and monkey show no Tp change...
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: