Closed Bug 1217939 Opened 4 years ago Closed 4 years ago

clean up nsContentUtils a little bit

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

Details

Attachments

(2 files)

The actual thing I wanted to do was removing nsContentUtils's dependence on
gfx's Units.h.  But since nsContentUtils::ToWidgetPoint is used internally and
in nsDOMWindowUtils, my goal looks unattainable.  Still, there are a few useful
things to fix here nonetheless; maybe the next person who comes along will have
a better idea.
All it was doing was forwarding to the provided document, which is an
unnecessary layer of indirection.  (I suppose this might have been necessary in
the separate-libraries-for-everything days, but nowadays everything lives in
libxul, so...)
Attachment #8678262 - Flags: review?(bugs)
All callers of this function are in layout/base/ and it uses a lot of
things from nsLayoutUtils, so nsLayoutUtils seems like a better fit for
it.
Attachment #8678263 - Flags: review?(bugs)
Comment on attachment 8678262 [details] [diff] [review]
part 1 - remove nsContentUtils::GetViewportInfo

>+  /**
>+   * Retrieve information about the viewport as a data structure.
>+   * This will return information in the viewport META data section
>+   * of the document. This can be used in lieu of ProcessViewportInfo(),
>+   * which places the viewport information in the document header instead
>+   * of returning it directly.
>+   *
>+   * @param aDisplayWidth width of the on-screen display area for this
>+   * document, in device pixels.
>+   * @param aDisplayHeight height of the on-screen display area for this
>+   * document, in device pixels.
>+   *
>+   * NOTE: If the site is optimized for mobile (via the doctype), this
>+   * will return viewport information that specifies default information.
>+   */
>   virtual nsViewportInfo GetViewportInfo(const mozilla::ScreenIntSize& aDisplaySize) = 0;
The copy-paste comment obviously isn't right. It talks about non-existing arguments.
So, fix the comment. It could just mentioned in which coordinates pixels aDisplaySize is about.
Attachment #8678262 - Flags: review?(bugs) → review+
Comment on attachment 8678263 [details] [diff] [review]
part 2 - move nsContentUtils::GetSelectionBoundingRect to nsLayoutUtils

>+  /**
>+   * Takes a selection, and return selection's bounding rect which is relative
>+   * to its root frame.
Shouldn't it be 'returns', not 'return'
Attachment #8678263 - Flags: review?(bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/37d64688bf9d
https://hg.mozilla.org/mozilla-central/rev/d9f5c51cfa00
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.