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

Remap HTMLHtmlElement to HTMLElement #2

Closed
wants to merge 1 commit into from
Closed

Remap HTMLHtmlElement to HTMLElement #2

wants to merge 1 commit into from

Conversation

jdonaldson
Copy link

I saw that there was a conflict between these idl files:
HTMLElement
HTMLHtmlElement (which is mapped to HtmlElement)

The two externs have the same name with different capitalization. This causes problems resolving the types in other externs. One possible way of dealing with this is mapping HTMLHtmlElement to HTMLElement, since it only defines a few extra misc. fields. That's what this commit does. I'm able to successfully compile everything in the library now.

Read on for more details from the commit log:

This commit maps the HTMLHtmlElement to the HTMLElement type
HTMLHtmlElement is the root html element of a document. It
currently has two extra metadata fields: manifest and version.
Otherwise, it behaves like a normal HTMLElement.

The commit also includes a simple build.hxml that will
include all of the js.html classes, to ensure that the types
resolve correctly.

After re-generating the externs, I found that several classes
were no longer being created, are they still needed?
Here are the classes that would be deleted:
deleted: generated/js/html/IceCallback.hx
deleted: generated/js/html/IceCandidate.hx
deleted: generated/js/html/PeerConnection00.hx
deleted: generated/js/html/SessionDescription.hx

This commit maps the HTMLHtmlElement to the HTMLElement type
HTMLHtmlElement is the root html element of a document.  It
currently has two extra metadata fields: manifest and version.
Otherwise, it behaves like a normal HTMLElement.

The commit also includes a simple build.hxml that will
include all of the js.html classes, to ensure that the types
resolve correctly.

After re-generating the externs, I found that several classes
were no longer being created, are they still needed?
Here are the classes that would be deleted:
deleted:    generated/js/html/IceCallback.hx
deleted:    generated/js/html/IceCandidate.hx
deleted:    generated/js/html/PeerConnection00.hx
deleted:    generated/js/html/SessionDescription.hx
@aduros
Copy link
Owner

aduros commented Dec 2, 2012

Besides being a little confusing, what sorts of problems are there from having HTMLElement and HtmlElement? All the externs seem to already compile fine for me.

The deleted and modified classes are probably because we're using different versions of webkit.

@jdonaldson
Copy link
Author

First of all, I should've mentioned that this pull request is primarily for discussion. Please drop this if you see a better approach.

Second of all, I should've mentioned that this problem is due to limitations in how OSX handles file names. The main issue is that OSX is case-insensitive and case-preserving. This means that the file names will preserve capitalization, but that you can't have two files with the same name and different capitalization. You can see related stackoverflow posts for more background on this issue, and how it affects cvs:
http://stackoverflow.com/questions/10523849/git-changing-capitalization-of-filenames

The pull request tries to address by effectively changing references of HTMLHtmlElement to HTMLElement. But, this is kind of a quick hack just so that I could see what you've done. There might be better approaches using namespaces, and I think that is pretty important to get right. So, I wanted to take a step back and look at the bigger picture.

I know that NC wanted to provide namespaces for audio, etc. One obvious technique might be to designate some of the extern file name prefixes as having namespaces:

AudioBuffer.idl => audio/AudioBuffer.hx
AudioBufferView.idl => audio/AudioBufferView.hx

Just by glancing through the IDL list, it looks like the following prefixes would be good namespaces:
webgl, svg, svg.pathseg, svg.fontface, svg.fe, audio, dom, css, html, idb, oes, rtc, media, sql, speech, file, directory, ...

The other issue is whether or not to modify the original typedef name to remove the prefix:
HTMLElement => html.Element

This seems to make sense for most html typdefs. However, consider the following list:
Element, HTMLElement, HTMLHtmlElement

Currently, you're putting Element into the html namespace, along with HTMLElement. Anything else that has the HTML prefix has this prefix stripped, including HTMLHtmlElement. In addition to causing confusion, this causes the file name problem mentioned above.

One suggestion might be to leave Element.idl out of the html namespace, and to remove the HTML prefix from everything that goes in that namespace.
Element.idl => Element
HTMLElement => html.Element
HTMLHtmlElement => html.HtmlElement

For the rest of the namespace mappings, I think it's better to preserve the prefix in the generated file name :
DomError.idl => dom.DomError
instead of: dom.Error

This is mainly because there's a lot of duplicate class names (with different namespaces) that would get created this way. E.g., you would have file.Error, media.Error, etc. It would get confusing to deal with this when importing these types, etc.

Let me know if you want to post this back to the list for more thoughts, and thanks for taking the time to put this together.

Best,
-Justin

@aduros
Copy link
Owner

aduros commented Dec 2, 2012

Oh, the filename thing is a good point! It looks like what Dart does is merge Element and HTMLElement. That seems reasonable... I've never needed to operate on an Element in JS that wasn't also an HTMLElement.

So the inheritance chain for HTMLHtmlElement.idl would be HtmlElement → Element → Node → EventTarget.

I'm only planning to namespace SVG, WebGL and Web Audio for now. Probably by having a function somewhere that looks at the filename and the parent class (for all AudioNode subclasses which aren't prefixed).

@jdonaldson
Copy link
Author

Merging sounds like a good idea right now from what I can see. I'm nervous messing around with how the inheritance chain is exposed. There's a lot of functions that produce simple Element types in the IDL methods. But, I would guess the Dart folks know better than most on how they want to work with their own browser. Following their lead sounds like a good bet here.

Going with inheritance + prefix (rather than just the prefix) also sounds like the right approach for specifying haxe-specific namespaces.

Related: There's another related SO thread here. They don't address "why" there is a difference, just that it exists:
http://stackoverflow.com/questions/6581680/whats-the-difference-between-htmlelement-and-element

@aduros aduros closed this in 52b721f Dec 10, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants