Closed
Bug 888318
Opened 11 years ago
Closed 11 years ago
Use CSSIntPoint for nsGlobalWindow::ScrollTo
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla25
People
(Reporter: Ms2ger, Assigned: Ms2ger)
References
Details
Attachments
(1 file)
(deleted),
patch
|
mrbkap
:
review+
kats
:
feedback+
|
Details | Diff | Splinter Review |
No description provided.
Attachment #768975 -
Flags: review?(bugmail.mozilla)
Comment 1•11 years ago
|
||
Comment on attachment 768975 [details] [diff] [review] Patch v1 Review of attachment 768975 [details] [diff] [review]: ----------------------------------------------------------------- Looks ok to me. Again I would prefer to also get a r+ from somebody who is a peer or owner of this code. ::: dom/base/nsGlobalWindow.cpp @@ +6032,5 @@ > +} > + > +void > +nsGlobalWindow::ScrollTo(const CSSIntPoint& aScroll) > +{ I think the compiler would be able to optimize this better if you changed the signature to ScrollTo(CSSIntPoint aScroll) and didn't copy it to a local "scroll" variable inside the body. That way it should be able to get away without copying it at all at call sites, whereas here it must make the copy in this compilation unit regardless.
Attachment #768975 -
Flags: review?(bugmail.mozilla) → feedback+
Assignee | ||
Updated•11 years ago
|
Attachment #768975 -
Flags: review?(mrbkap)
Comment 2•11 years ago
|
||
Comment on attachment 768975 [details] [diff] [review] Patch v1 Review of attachment 768975 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/nsGlobalWindow.cpp @@ +6039,5 @@ > > if (sf) { > // Here we calculate what the max pixel value is that we can > // scroll to, we do this by dividing maxint with the pixel to > // twips conversion factor, and substracting 4, the 4 comes from While you're here, want to fix substracting -> subtracting?
Attachment #768975 -
Flags: review?(mrbkap) → review+
Assignee | ||
Comment 3•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6a1a37e55104
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•