918189 - Implement Node.convertPoint/Rect/QuadFromNode

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

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

Based on http://lists.w3.org/Archives/Public/www-style/2013Aug/0607.html and followup.

Comment 1

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

Attachment: Part 1: Implement Node.convertPoint/Rect/QuadFromNode

Comment 2

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

Attachment: Part 2: Implement constructors for DOMRect and DOMQuad

Comment 3

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

Attachment: Part 3: Refactor getBoxQuads test so it can test convertPoint/Rect/Quad as well

Comment 4

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

Attachment: Part 4: Add tests for convertPoint/Rect/Quad

Comment 5

[Masatoshi Kimura [:emk]]

Will these interfaces be exposed on the global immediately even without a draft spec?

Comment 6

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

A draft spec is imminent.

Comment 7

[Mats Palmgren (inactive)]

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.

Comment 8

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

Attachment: Part 1: Implement GeometryUtils.convertPointFromNode, convertRectFromNode, and convertQudFromNode918189-implement

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.

Comment 9

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

Attachment: Part 2: Add tests for convertPoint/Rect/QuadFromNode

Comment 10

[Mats Palmgren (inactive)]

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.

Comment 11

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

(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 12

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

Attachment: Part 1: Implement GeometryUtils ConvertQuad/Rect/PointFromNode

Comment 13

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

Attachment: Part 1.5: Implement GeometryUtils.convertPointFromNode, convertRectFromNode, and convertQuadFromNode

Comment 14

[Mats Palmgren (inactive)]

Comment on attachment 8388511 [details] [diff] [review]
Part 1: Implement GeometryUtils ConvertQuad/Rect/PointFromNode

r=mats

Comment 15

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

Attachment: Part 1.5: Implement GeometryUtils.convertPointFromNode, convertRectFromNode, and convertQuadFromNode

Comment 16

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/f7e676be8fa0
https://hg.mozilla.org/integration/mozilla-inbound/rev/e60fe593236d
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f3187619c9e

Comment 17

[Carsten Book [:Tomcat]]

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

Comment 18

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

Relanded with test fixed.
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e6446f6d0e6
https://hg.mozilla.org/integration/mozilla-inbound/rev/48edbd324d0f
https://hg.mozilla.org/integration/mozilla-inbound/rev/716f942b5798

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Backed out for bustage.
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9f011ba0f5e

https://tbpl.mozilla.org/php/getParsedLog.php?id=36462189&tree=Mozilla-Inbound

Comment 20

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/e8b54afa1a7b
https://hg.mozilla.org/integration/mozilla-inbound/rev/071acee02c25
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c538fcbb578

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e8b54afa1a7b
https://hg.mozilla.org/mozilla-central/rev/071acee02c25
https://hg.mozilla.org/mozilla-central/rev/8c538fcbb578

Comment 22

[Daniel Holbert [:dholbert]]

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