Closed Bug 1281800 Opened 3 years ago Closed 3 years ago

ctx.fillText(text, x, y, null) behavior is incorrect

Categories

(Core :: Canvas: 2D, defect)

x86_64
All
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: xidachen, Assigned: vliu)

Details

Attachments

(3 files, 1 obsolete file)

Attached file fillText.html
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36

Steps to reproduce:

Open the attached html file in firefox nightly build


Actual results:

"Hello world C" is displayed.


Expected results:

"Hello world C" should not be displayed
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0

I have tested this issue on Ubuntu 14.04 x32, Ubuntu 16.04 x64, Mac OS X 10.9 and Windows 10 x64 with the latest Firefox release (47.0) and the latest Nightly (50.0a1-20160627030215) and managed to reproduce it.
When opening the testcase provided in the description you can observe that "Hello world C" is displayed.
Status: UNCONFIRMED → NEW
Component: Untriaged → Canvas: 2D
Ever confirmed: true
OS: Unspecified → All
Product: Firefox → Core
Hardware: Unspecified → x86_64
I can also reproduce it on my Mac OS 10.11 through I am not sure the reasonable behavior at this moment, but I will take it to look into this issue.
Assignee: nobody → vliu
From the spec in [1], the API should return an empty array if maxWidth was provided but is less than or equal to zero or equal to NaN.

[1]: https://html.spec.whatwg.org/multipage/scripting.html#dom-context-2d-filltext
Hi Jeff,
Could you please help me to review this patch? Thanks.
Attachment #8766971 - Flags: review?(jmuizelaar)
Comment on attachment 8766971 [details] [diff] [review]
The API should return an empty array if maxWidth was provided but is less than or equal to zero or equal to NaN.

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

Seems reasonable. Can you make sure there's a web platform test for this? https://github.com/w3c/web-platform-tests/tree/master/2dcontext
Attachment #8766971 - Flags: review?(jmuizelaar) → review+
Hi Jeff,
The attached patch for adding wpt for fillText(). I found we already have wpt for zero/negative. I noly remove their *.ini file
since they all expect ture for the result. Besides, I add new test for NaN case. Could you please have review for them? Thanks.


Hi Ms2ger,
As talked on IRC, could you please also have review for the test? Really thanks.
Attachment #8769963 - Flags: review?(jmuizelaar)
Attachment #8769963 - Flags: review?(Ms2ger)
Comment on attachment 8769963 [details] [diff] [review]
Add wpt for fillText() in Canvas. r=jmuizelaar, Ms2ger

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

The test will not be run; do `./mach test-wpt --manifest-update 2dcontext/drawing-text-to-the-canvas/2d.text.draw.fill.maxWidth.NaN.html`.
Attachment #8769963 - Flags: review?(Ms2ger)
Comment on attachment 8769963 [details] [diff] [review]
Add wpt for fillText() in Canvas. r=jmuizelaar, Ms2ger

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

The actual content of the test looks fine. I'll defer to Ms2ger for the infrastructure surrounding it.
Attachment #8769963 - Flags: review?(jmuizelaar) → review+
Hi Ms2ger,
The modification for MANIFEST.json was attached in patch. Could you please have a review? Thanks
Attachment #8770481 - Flags: review?(Ms2ger)
Attachment #8769963 - Attachment is obsolete: true
Comment on attachment 8770481 [details] [diff] [review]
Add wpt for fillText() in Canvas. r=jmuizelaar, Ms2ger

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

Thanks, LGTM. Do fold this into the patch that changes the code, though.
Attachment #8770481 - Flags: review?(Ms2ger) → review+
Pushed by vliu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1c09aef633ab
The API should return an empty array if maxWidth was provided but is less than or equal to zero or equal to NaN. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/2d8e504f0e0c
Add wpt for fillText() in Canvas. r=jmuizelaar, Ms2ger
https://hg.mozilla.org/mozilla-central/rev/1c09aef633ab
https://hg.mozilla.org/mozilla-central/rev/2d8e504f0e0c
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
(In reply to :Ms2ger from comment #10)
> Do fold this into the patch that changes the code, though.

*sigh*
You need to log in before you can comment on or make changes to this bug.