Closed Bug 918189 Opened 7 years ago Closed 6 years ago

Implement Node.convertPoint/Rect/QuadFromNode

Categories

(Core :: Layout, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: roc, Assigned: roc)

References

(Blocks 3 open bugs)

Details

(Keywords: dev-doc-needed)

Attachments

(3 files, 6 obsolete files)

15.71 KB, patch
mats
: review+
Details | Diff | Splinter Review
9.54 KB, patch
mats
: review+
Details | Diff | Splinter Review
10.76 KB, patch
jst
: review+
Details | Diff | Splinter Review
Will these interfaces be exposed on the global immediately even without a draft spec?
A draft spec is imminent.
Comment on attachment 807614 [details] [diff] [review]
Part 1: Implement Node.convertPoint/Rect/QuadFromNode

Please ask again when the www-style discussion has settled or a spec
is available.
Attachment #807614 - Flags: review?(matspal)
Attachment #807615 - Flags: review?(matspal)
Attachment #807616 - Flags: review?(matspal)
Attachment #807617 - Flags: review?(matspal)
These APIs are in a similar situation to getBoxQuads. We have consensus on how they should work, possibly except for some corner cases that haven't been discussed.
Attachment #807614 - Attachment is obsolete: true
Attachment #807615 - Attachment is obsolete: true
Attachment #807616 - Attachment is obsolete: true
Attachment #807617 - Attachment is obsolete: true
Attachment #8356417 - Flags: review?(matspal)
Comment on attachment 8356417 [details] [diff] [review]
Part 1: Implement GeometryUtils.convertPointFromNode, convertRectFromNode, and convertQudFromNode918189-implement

I'm only reviewing the layout/* files.
The rest should be reviewed by a DOM peer.

> layout/base/GeometryUtils.cpp 

It's unfortunate with duplicated code for OwningGeometryNode and
GeometryNode, is there no way to get a GeometryNode out of the
former so they can share some code?

For TransformCSSPoints - same comments as in bug 917755 comment 37.
Attachment #8356417 - Flags: review?(matspal) → review-
Attachment #8356418 - Flags: review?(matspal) → review+
(In reply to Mats Palmgren (:mats) from comment #10)
> It's unfortunate with duplicated code for OwningGeometryNode and
> GeometryNode, is there no way to get a GeometryNode out of the
> former so they can share some code?

I don't think so, at least not with significant changes to WebIDL codegen.
Comment on attachment 8388511 [details] [diff] [review]
Part 1: Implement GeometryUtils ConvertQuad/Rect/PointFromNode

r=mats
Attachment #8388511 - Flags: review?(matspal) → review+
Attachment #8389634 - Flags: review?(jst) → review+
Hi,

sorry Roc had to backout this changes as part of https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=12e871cf1100 for test failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=36383057&tree=Mozilla-Inbound - not sure if this is a problem in the test or the other checkins
Blocks: 1107559
Comment on attachment 8389634 [details] [diff] [review]
Part 1.5: Implement GeometryUtils.convertPointFromNode, convertRectFromNode, and convertQuadFromNode

>+++ b/modules/libpref/src/init/all.js
>@@ -1801,16 +1801,23 @@ pref("layout.css.DOMQuad.enabled", true)
> 
> // Is support for GeometryUtils.getBoxQuads enabled?
> #ifdef RELEASE_BUILD
> pref("layout.css.getBoxQuads.enabled", false);
> #else
> pref("layout.css.getBoxQuads.enabled", true);
> #endif
> 
>+// Is support for GeometryUtils.getBoxQuads enabled?
>+#ifdef RELEASE_BUILD
>+pref("layout.css.convertFromNode.enabled", false);

I just ran across this section of all.js & noticed that there's a copy-paste typo in the comment here - the latter "Is support for..." comment needs s/getBoxQuads/convert*FromNode/.

Pushed a comment-only followup to make that tweak:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/52e1fb0b5576
You need to log in before you can comment on or make changes to this bug.