Bug 203646 - [SVG2] Add the 'orient' property of the interface SVGMarkerElement
Summary: [SVG2] Add the 'orient' property of the interface SVGMarkerElement
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords: InRadar
Depends on:
Blocks: 191292 200143
  Show dependency tree / graph
 
Reported: 2019-10-30 16:43 PDT by Said Abou-Hallawa
Modified: 2019-12-04 10:48 PST (History)
16 users (show)

See Also:

Attachments
Patch (20.75 KB, patch)
2019-10-30 16:49 PDT, Said Abou-Hallawa 		no flags Details | Formatted Diff | Diff
Patch (33.69 KB, patch)
2019-11-13 13:26 PST, Said Abou-Hallawa 		no flags Details | Formatted Diff | Diff
Patch (35.11 KB, patch)
2019-11-13 16:03 PST, Said Abou-Hallawa 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Said Abou-Hallawa 2019-10-30 16:43:25 PDT 
The spec link is https://www.w3.org/TR/SVG2/painting.html#InterfaceSVGMarkerElement
Comment 1 Said Abou-Hallawa 2019-10-30 16:49:31 PDT 
Created attachment 382391 [details]
Patch
Comment 2 Said Abou-Hallawa 2019-11-13 13:26:40 PST 
Created attachment 383488 [details]
Patch
Comment 3 Simon Fraser (smfr) 2019-11-13 14:54:26 PST 
Comment on attachment 383488 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=383488&action=review

> Source/WebCore/svg/SVGMarkerElement.h:70
> +    void setOrientToAngle(SVGAngle&);

const SVGAngle& or Ref<SVGAngle>&& ? Could a null angle indicate "auto"?

> Source/WebCore/svg/SVGMarkerTypes.h:85
> +    static unsigned highestEnumValue() { return SVGMarkerOrientAngle; }

Deserves a comment to explain why it's not SVGMarkerOrientAutoStartReverse
Comment 4 Said Abou-Hallawa 2019-11-13 16:03:20 PST 
Created attachment 383508 [details]
Patch
Comment 5 Said Abou-Hallawa 2019-11-13 16:06:48 PST 
Comment on attachment 383488 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=383488&action=review

>> Source/WebCore/svg/SVGMarkerElement.h:70
>> +    void setOrientToAngle(SVGAngle&);
> 
> const SVGAngle& or Ref<SVGAngle>&& ? Could a null angle indicate "auto"?

Argument type was changed to const SVGAngle&. Some methods of SVGAngle had to be made const.

>> Source/WebCore/svg/SVGMarkerTypes.h:85
>> +    static unsigned highestEnumValue() { return SVGMarkerOrientAngle; }
> 
> Deserves a comment to explain why it's not SVGMarkerOrientAutoStartReverse

A comment was added in SVGMarkerTypes.h where the enum value SVGMarkerOrientAutoStartReverse is defined.
Comment 6 WebKit Commit Bot 2019-11-13 18:21:55 PST 
Comment on attachment 383508 [details]
Patch

Clearing flags on attachment: 383508

Committed r252444: <https://trac.webkit.org/changeset/252444>
Comment 7 WebKit Commit Bot 2019-11-13 18:21:56 PST 
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2019-11-13 18:22:23 PST 
<rdar://problem/57176727>
Comment 9 Darin Adler 2019-12-04 10:48:15 PST 
Comment on attachment 383508 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=383508&action=review

> Source/WebCore/svg/SVGMarkerElement.h:67
> +    String orient() const;
> +    void setOrient(const String&);

Can we use [Reflect] and omit these?

> Source/WebCore/svg/SVGMarkerElement.idl:45
> +    attribute DOMString orient;

Can we use [Reflect] here?