Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

getContext()'s second argument #9194

Merged
merged 2 commits into from
Feb 10, 2020
Merged

getContext()'s second argument #9194

merged 2 commits into from
Feb 10, 2020

Conversation

annevk
Copy link
Member

@annevk annevk commented Jan 25, 2018 •
edited by wpt-pr-bot
Loading

See whatwg/html#595 and whatwg/html#3370 for context.

@annevk annevk requested a review from gsnedders January 25, 2018 10:51
annevk added a commit to whatwg/html that referenced this pull request Jan 25, 2018
@annevk
Simplify getContext() and make it throw less
5137bbf 
In practice getContext() only has two arguments. And in practice some
handling of the second argument is shared across the various rendering
context types. This aligns the standard with that practice.

This also obviates the need for dedicated coercion algorithms for "2d"
and "bitmaprender".

This change also fixes various minor nits found along the way, such as
the inconsistent spelling of contextId (was sometimes contextType).

Tests: web-platform-tests/wpt#9194.

Fixes #595.
@tobie
Copy link
Contributor

tobie commented Jan 25, 2018

Summoning @wpt-pr-bot.

@annevk
Copy link
Member Author

annevk commented Jan 25, 2018

Someone else will have to run update-built-tests.sh since I'm not super keen on installing virtualenv and such.

annevk added a commit to whatwg/html that referenced this pull request Jan 26, 2018
@annevk
Simplify getContext() and make it throw less
9249592 
In practice getContext() only has two arguments. And in practice some
handling of the second argument is shared across the various rendering
context types. This aligns the standard with that practice.

This also obviates the need for dedicated coercion algorithms for "2d"
and "bitmaprender".

This change also fixes various minor nits found along the way, such as
the inconsistent spelling of contextId (was sometimes contextType).

Tests: web-platform-tests/wpt#9194 and
web-platform-tests/wpt#9205.

Fixes #595.
alice pushed a commit to alice/html that referenced this pull request Jan 8, 2019
@annevk
Simplify getContext() and make it throw less
8533f87 
In practice getContext() only has two arguments. And in practice some
handling of the second argument is shared across the various rendering
context types. This aligns the standard with that practice.

This also obviates the need for dedicated coercion algorithms for "2d"
and "bitmaprender".

This change also fixes various minor nits found along the way, such as
the inconsistent spelling of contextId (was sometimes contextType).

Tests: web-platform-tests/wpt#9194 and
web-platform-tests/wpt#9205.

Fixes whatwg#595.
@gsnedders gsnedders closed this Jan 24, 2020
@gsnedders gsnedders deleted the canvas/getContext-second-argument branch January 24, 2020 18:02
@gsnedders gsnedders restored the canvas/getContext-second-argument branch January 24, 2020 18:42
@Hexcles Hexcles reopened this Jan 24, 2020
@annevk annevk force-pushed the canvas/getContext-second-argument branch from d45dfd0 to 6217c19 Compare February 7, 2020 16:52
@annevk
Copy link
Member Author

annevk commented Feb 7, 2020 •
edited
Loading

Can someone help me out with getting 2dcontext/tools/gentest.py to run? I'm missing cairo, but pip install pycairo and easy_install pycairo are failing, possibly because it no longer supports p2? (Alternatively, if you're willing to run it and push a fixup commit, that'd be great!)

@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-9194 February 7, 2020 17:00 Inactive
@sideshowbarker
Copy link
Contributor

sideshowbarker commented Feb 8, 2020 •
edited
Loading

Can someone help me out with getting 2dcontext/tools/gentest.py to run? I'm missing cairo, but pip install pycairo and easy_install pycairo are failing, possibly because it no longer supports p2?

You might be able to get past that with brew by doing brew install py2cairo.

But even when I do that in my environment, and run python ./2dcontext/tools/gentest.py, I get IOError: [Errno 2] No such file or directory: 'templates.yaml'

Note: I see it’s necessary to actually cd into the 2dcontext/tools subdirectory before trying to run the gentest.py script. Ideally that script should be invocable/runnable from the root of the repo (as pretty much all are other test-file-generation scripts were intentionally written to be).

@sideshowbarker
Copy link
Contributor

(Alternatively, if you're willing to run it and push a fixup commit, that'd be great!)

Done. html/semantics/embedded-content/the-canvas-element/2d.getcontext.extraargs.html seems to be the only (generated) test file that ends up with a substantive change. I guess that’s expected. (The rest of the affected files only have the update This test has been generated by /2dcontext/tools/gentest.py. comment change.)

@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-9194 February 8, 2020 12:07 Inactive
@sideshowbarker sideshowbarker force-pushed the canvas/getContext-second-argument branch from c79e410 to 507b854 Compare February 10, 2020 02:16
@sideshowbarker
Copy link
Contributor

Based on the IRC comment at https://w3.logbot.info/testing/20200208#c542306, I went ahead and split this into two commits (and rewrote the history on the branch and force-pushed it here).

Please doublecheck it all.

@annevk
Copy link
Member Author

annevk commented Feb 10, 2020

LGTM, but I cannot approve as I wrote this.

@jgraham jgraham merged commit 92e8b40 into master Feb 10, 2020
@jgraham jgraham deleted the canvas/getContext-second-argument branch February 10, 2020 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsnedders gsnedders Awaiting requested review from gsnedders

@AmeliaBR AmeliaBR Awaiting requested review from AmeliaBR

@fserb fserb Awaiting requested review from fserb

@kenrussell kenrussell Awaiting requested review from kenrussell

@domenic domenic Awaiting requested review from domenic

@foolip foolip Awaiting requested review from foolip

@jdm jdm Awaiting requested review from jdm

@jgraham jgraham Awaiting requested review from jgraham

@zcorpan zcorpan Awaiting requested review from zcorpan

@zqzhang zqzhang Awaiting requested review from zqzhang

Assignees

@fserb fserb

Labels
2dcontext html offscreen-canvas wg-html
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@annevk @tobie @sideshowbarker @fserb @gsnedders @jgraham @Hexcles @wpt-pr-bot