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.

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

Update the canvas webidl to match the spec #9443

Closed
jdm opened this issue Jan 27, 2016 · 14 comments
Closed

Update the canvas webidl to match the spec #9443

jdm opened this issue Jan 27, 2016 · 14 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented Jan 27, 2016

Our CanvasRenderingContext2D.webidl is based on an older revision before the spec started grouping related APIs together in different interfaces. We should update it to match the spec more closely. There should be no functional change and everything should keep compiling exactly as it does today. Some features like Exposed=(Window,Worker) will need to be commented out because we don't support them yet.

Code: components/script/dom/webidls/CanvasRenderingContext2D.webidl
Spec: https://html.spec.whatwg.org/multipage/scripting.html#2dcontext

@J2R5M3
Copy link

@J2R5M3 J2R5M3 commented Jan 27, 2016

Looking to get involved, this would be my first contribution to open source anything,ever.

Don't really know where to start so a welcome shove off the correct cliff would be appreciated, I'll find a parachute on the way down.

On a side note I've been have some issues compiling the source :
error: linking withccfailed: exit code: 1
"lots of noise"
then

note: /usr/bin/ld: cannot find -lEGL
collect2: error: ld returned 1 exit status

Done a fair bit of googling but haven't really been able to find much.

@jdm
Copy link
Member Author

@jdm jdm commented Jan 27, 2016

@J2R5M3 Welcome! It appears you're missing a dependency, and presumably you're not on a debian-based linux as that would be covered by the "libegl1-mesa-dev" in README.md. You may want to try installing "mesa-libEGL-devel" if you're on Fedora? Hard to give better suggestions without more information.

As for how to get started, this is really about cleaning up the contents of the webidl file I linked in the original description to match the WebIDL interfaces that are present in the specification that's also linked previously. Does that help at all?

@J2R5M3
Copy link

@J2R5M3 J2R5M3 commented Jan 28, 2016

mostly, just was wondering about any sources I could reference regarding which features are supported and which aren't

@jdm
Copy link
Member Author

@jdm jdm commented Jan 28, 2016

All I can recommend is to use the existing webidl files as reference (ie. note what is already commented out).

@gmalecha
Copy link
Contributor

@gmalecha gmalecha commented Feb 1, 2016

Has this bug been claimed?

@nox
Copy link
Member

@nox nox commented Feb 1, 2016

@gmalecha It seems @J2R5M3 started looking at it.

@J2R5M3 Do you need help with this?

@J2R5M3
Copy link

@J2R5M3 J2R5M3 commented Feb 2, 2016

@gmalecha

I've been kinda swamped with a wave of assignments for school and work, if you want to lend a hand or even take over that's perfectly fine with me,

I had hoped to get involved but it is the case that my current schedule can't handle anything additional right now at least for the next 4-5 days, priorities and such.

@gmalecha
Copy link
Contributor

@gmalecha gmalecha commented Feb 2, 2016

@J2R5M3 I'm happy to let you take care of it. Should it be marked assigned?

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Feb 2, 2016

Okay, just let us know of any updates! @J2R5M3

@KiChjang KiChjang added the C-assigned label Feb 2, 2016
@krstoff
Copy link

@krstoff krstoff commented Mar 1, 2016

This has been left idle for about a month and I would like to try fixing it. Any objections?

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Mar 1, 2016

Since @J2R5M3 doesn't seem to be active lately, I think you can go ahead and work on this issue @krstoff.

@gmalecha
Copy link
Contributor

@gmalecha gmalecha commented Mar 1, 2016

I believe that I already have an implementation of it (though it is about a month old now). I can update it to the latest code and see if it still works.

@krstoff
Copy link

@krstoff krstoff commented Mar 1, 2016

Okay, never mind then. :) Let me know if anything changes @gmalecha

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Mar 1, 2016

@gmalecha Are you planning to make a PR for this issue? If not, then @krstoff can go ahead and work on this.

gmalecha added a commit to gmalecha/servo that referenced this issue Mar 1, 2016
bors-servo added a commit that referenced this issue Mar 1, 2016
…, r=jdm

updating the CanvasRenderingContext2D to match the spec

- fixes #9443

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9823)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Mar 1, 2016
…, r=jdm

updating the CanvasRenderingContext2D to match the spec

- fixes #9443

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9823)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

6 participants
You can’t perform that action at this time.