The default bug view has changed. See this FAQ.

Implement canvasRenderingContext2D.get/setLineDash

RESOLVED FIXED in mozilla27

Status

()

Core
Canvas: 2D
RESOLVED FIXED
5 years ago
8 months ago

People

(Reporter: peterv, Assigned: Rik Cabanier)

Tracking

(Blocks: 2 bugs, {dev-doc-complete, html5})

Trunk
mozilla27
dev-doc-complete, html5
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(relnote-firefox 27+)

Details

(Whiteboard: [parity with Safari, Chrome and IE], URL)

Attachments

(2 attachments, 5 obsolete attachments)

Comment hidden (empty)
(Assignee)

Comment 1

4 years ago
Created attachment 813352 [details] [diff] [review]
canvaslinedash.patch
(Assignee)

Comment 2

4 years ago
Created attachment 813353 [details] [diff] [review]
Add support for set/getLineDash and dash offset
Attachment #813352 - Attachment is obsolete: true
(Assignee)

Comment 3

4 years ago
Created attachment 813354 [details] [diff] [review]
Add support for set/getLineDash and dash offset
Attachment #813353 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Attachment #813354 - Flags: review?(roc)
(Assignee)

Comment 4

4 years ago
Not sure about removing the moz methods. That would break current sites...
(Assignee)

Comment 5

4 years ago
try build: https://tbpl.mozilla.org/?tree=Try&rev=92fa748e253b
(Assignee)

Updated

4 years ago
Assignee: nobody → cabanier
Keywords: dev-doc-needed, html5
Whiteboard: [parity with Safari, Chrome and IE]
(Assignee)

Updated

4 years ago
Attachment #813354 - Flags: review?(roc) → review?(bas)
(Assignee)

Comment 6

4 years ago
Created attachment 815180 [details] [diff] [review]
Add support for set/getLineDash and dash offset
Attachment #815180 - Flags: review?(bas)
(Assignee)

Updated

4 years ago
Attachment #813354 - Attachment is obsolete: true
Attachment #813354 - Flags: review?(bas)

Comment 7

4 years ago
This Bug 768067, Bug 740284, Bug 683051 and Bug 809586 seem to overlap at least in part. Can somebody with the necessary knowledge (and privs) please clean up/merge and clarify the individual bugs & summaries?
Blocks: 802882
Comment on attachment 815180 [details] [diff] [review]
Add support for set/getLineDash and dash offset

Review of attachment 815180 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/canvas/src/CanvasRenderingContext2D.cpp
@@ +2856,5 @@
> +CanvasRenderingContext2D::SetLineDash(const mozilla::dom::AutoSequence<double>& mSegments) {
> +  FallibleTArray<mozilla::gfx::Float>& dash = CurrentState().dash;
> +  dash.Clear();
> +
> +  for(mozilla::dom::AutoSequence<double>::index_type x = 0; x < mSegments.Length(); x++) {

uint32_t type for 'x'.

@@ +2859,5 @@
> +
> +  for(mozilla::dom::AutoSequence<double>::index_type x = 0; x < mSegments.Length(); x++) {
> +    dash.AppendElement(mSegments[x]);
> +  }
> +  if(mSegments.Length()%2) { // If the number of elements is odd, concatenate again

Sapace after 'if' and 'for'.

@@ +2867,5 @@
> +  }
> +}
> +
> +void
> +CanvasRenderingContext2D::GetLineDash(nsTArray<double>& mSegments) const {

This parameter should be called aSegments

@@ +2871,5 @@
> +CanvasRenderingContext2D::GetLineDash(nsTArray<double>& mSegments) const {
> +  const FallibleTArray<mozilla::gfx::Float>& dash = CurrentState().dash;
> +  mSegments.Clear();
> +
> +  for(FallibleTArray<mozilla::gfx::Float>::index_type x = 0; x < dash.Length(); x++) {

Space after 'for'.

Also, you can make x 'uint32_t'. Easier to read.
Attachment #815180 - Flags: review?(bas) → review+
(Assignee)

Comment 9

4 years ago
Created attachment 817399 [details] [diff] [review]
Add support for set/getLineDash and dash offset
Attachment #815180 - Attachment is obsolete: true
(Assignee)

Comment 10

4 years ago
Updated patch per roc's comments.
Try server: https://tbpl.mozilla.org/?tree=Try&rev=e6dd13184378
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/b16182c733bb
Flags: in-testsuite+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b16182c733bb
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Blocks: 918589
relnote-firefox: --- → ?

Updated

4 years ago
Duplicate of this bug: 809586
(In reply to Rik Cabanier from comment #10)
> Updated patch per roc's comments.

The comments were not addressed.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 15

4 years ago
(In reply to :Ms2ger from comment #14)
> (In reply to Rik Cabanier from comment #10)
> > Updated patch per roc's comments.
> 
> The comments were not addressed.

I must have copied the wrong patch :-(
Will fix this asap
(Assignee)

Comment 16

4 years ago
Created attachment 823033 [details] [diff] [review]
Fix style
Attachment #823033 - Flags: review?(Ms2ger)
(Assignee)

Comment 17

4 years ago
Created attachment 823037 [details] [diff] [review]
Fix incorrect style
Attachment #823033 - Attachment is obsolete: true
Attachment #823033 - Flags: review?(Ms2ger)
Attachment #823037 - Flags: review?(Ms2ger)
Comment on attachment 823037 [details] [diff] [review]
Fix incorrect style

Review of attachment 823037 [details] [diff] [review]:
-----------------------------------------------------------------

This is r=roc in comment 8.
Attachment #823037 - Flags: review?(Ms2ger)
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
Blocks: 931643
Summary: Implement canvasRenderingContext2D.get/setLineDash and deprecate/remove mozDash → Implement canvasRenderingContext2D.get/setLineDash
https://hg.mozilla.org/integration/mozilla-inbound/rev/222175ec725f
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/222175ec725f
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED

Updated

3 years ago
relnote-firefox: ? → 27+
Reference:
https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D.getLineDash
https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D.setLineDash
https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D.lineDashOffset
Release notes:
https://developer.mozilla.org/en-US/Firefox/Releases/27#Interfaces.2FAPIs.2FDOM
Keywords: dev-doc-needed → dev-doc-complete
Blocks: 1119470

Updated

8 months ago
Depends on: 931389
You need to log in before you can comment on or make changes to this bug.