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

New definition of "H", "L", "M", "T", new feature FORMS (if needed) #56

Merged
merged 10 commits into from
Oct 14, 2024

Conversation

Clayblockunova
Copy link
Contributor

@Clayblockunova Clayblockunova commented Oct 8, 2024

afaik from MDN, document.forms+[] returns to "[object HTMLCollection]" in most browsers, but idk how this actually work in old Android webview, so I created a new feature just in case (idk how to simulate, tho). btw, afaik, forms have a relatively shorter code in those similar properties of document, but idk is that the shortest.

@Clayblockunova
Copy link
Contributor Author

what should I do?

@fasttime
Copy link
Owner

fasttime commented Oct 9, 2024

Thanks! I just need some time to look into this.

@fasttime
Copy link
Owner

fasttime commented Oct 9, 2024

It looks like there are already better definitions for all of the characters. Am I missing something?

@Clayblockunova
Copy link
Contributor Author

It looks like there are already better definitions for all of the characters. Am I missing something?

you mean there is something better than forms?

@fasttime
Copy link
Owner

I think so. At least I could not find a case where the new definitions produce shorter code for some combination of browsers.

@Clayblockunova
Copy link
Contributor Author

I think so. At least I could not find a case where the new definitions produce shorter code for some combination of browsers.

the expected combination is browsers except old Android web view.

@fasttime
Copy link
Owner

FORMS is available in Android Browser 4.0 and 4.1, but we still need the feature because it's not available in Node.js.

@Clayblockunova
Copy link
Contributor Author

can we just treat that's part of ANY_DOCUMENT?

@fasttime
Copy link
Owner

can we just treat that's part of ANY_DOCUMENT?

I think it's easier to treat it as a separate feature. If we make ANY_DOCUMENT more specific we will also need to redefine DOCUMENT, and the emulation logic for both features.

@Clayblockunova
Copy link
Contributor Author

and, the "V" definition based on document.createElement relies on ANY_DOCUMENT now. should we create a separate feature for this?

@Clayblockunova
Copy link
Contributor Author

or should I add new simulation now? what should I do?

@fasttime
Copy link
Owner

and, the "V" definition based on document.createElement relies on ANY_DOCUMENT now. should we create a separate feature for this?

Yes, that would be better. I will split a new feature.

src/lib/features.js Outdated Show resolved Hide resolved
src/lib/features.js Outdated Show resolved Hide resolved
Clayblockunova and others added 2 commits October 12, 2024 10:57
Co-authored-by: Francesco Trotta <github@fasttime.org>
Co-authored-by: Francesco Trotta <github@fasttime.org>
@Clayblockunova
Copy link
Contributor Author

Clayblockunova commented Oct 12, 2024

And, how to simulate?

src/lib/features.js Outdated Show resolved Hide resolved
@fasttime
Copy link
Owner

And, how to sumulate?

To emulate document.forms we can add a new property to the stub for document in the function makeEmuFeatureDocument:

document = { createElement: createElement, nodeName: '#document' };

for example:

                document =
                {
                    createElement:  createElement,
                    forms:          '[object HTMLCollection]',
                    nodeName:       '#document',
                };

Clayblockunova and others added 3 commits October 13, 2024 18:36
Co-authored-by: Francesco Trotta <github@fasttime.org>
Copy link
Owner

@fasttime fasttime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, thanks!

@fasttime fasttime merged commit fa66053 into fasttime:main Oct 14, 2024
59 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants