Skip to content

/ servo

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

Jump to bottom

Treat 'undefined' passed to an optional argument as "missing" #8813

Closed
frewsxcv opened this issue Dec 4, 2015 · 9 comments
Closed

Treat 'undefined' passed to an optional argument as "missing" #8813

frewsxcv opened this issue Dec 4, 2015 · 9 comments
Assignees
@frewsxcv
Labels
A-content/bindings C-assigned

Comments

@frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Dec 4, 2015

Servo:

> document.implementation.createHTMLDocument(undefined).title
"undefined"

Firefox:

> document.implementation.createHTMLDocument(undefined).title
""

Chrome:

> document.implementation.createHTMLDocument(undefined).title
""

Is this a spec issue?
@frewsxcv
Copy link
Member Author

@frewsxcv frewsxcv commented Dec 4, 2015

/cc @Ms2ger
@eefriedman
Copy link
Contributor

@eefriedman eefriedman commented Dec 4, 2015

The spec is perfectly clear; the interface is [NewObject] Document createHTMLDocument(optional DOMString title);, and the Web IDL spec says that if undefined is specified as an argument, it gets converted to the string "undefined". Servo's behavior matches the spec.

It's possible the spec needs to be changed to reflect reality.
@frewsxcv frewsxcv mentioned this issue Dec 4, 2015
Closed
@frewsxcv
Copy link
Member Author

@frewsxcv frewsxcv commented Dec 4, 2015

whatwg/dom#124
@frewsxcv frewsxcv closed this Dec 4, 2015
@Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented Dec 4, 2015

@eefriedman is incorrect. Passing undefined to an optional argument is the same as not passing it at all. This behaviour is hidden in https://heycam.github.io/webidl/#dfn-overload-resolution-algorithm (currently step 10.4).
@Ms2ger Ms2ger reopened this Dec 4, 2015
@Ms2ger Ms2ger changed the title Cross-browser inconsistencies with createHTMLDocument Treat undefined passed to an optional argument as "missing" Dec 4, 2015
@Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented Dec 4, 2015

https://bugzilla.mozilla.org/show_bug.cgi?id=882541
@frewsxcv
Copy link
Member Author

@frewsxcv frewsxcv commented Dec 4, 2015

Potentially related to #6558
@frewsxcv frewsxcv changed the title Treat undefined passed to an optional argument as "missing" Treat 'undefined' passed to an optional argument as "missing" Dec 4, 2015
@eefriedman
Copy link
Contributor

@eefriedman eefriedman commented Dec 4, 2015

Oops, I was looking at the wrong version of the WebIDL specification. Sorry about that.
@frewsxcv
Copy link
Member Author

@frewsxcv frewsxcv commented Dec 5, 2015

Consider this example:

  1. Navigate to http://meow.com
  2. Enter location.replace(undefined) in the JS console

In Firefox, Chrome, and Safari, this redirects to http://meow.com/undefined. According to the spec, should location.replace(undefined) be treated identical to location.replace()? If so, this seems like it goes against what the norm already do, and I imagine having the major vendors transition could cause quite a bit of pain for people.
@frewsxcv
Copy link
Member Author

@frewsxcv frewsxcv commented Dec 5, 2015

Nevermind. I missed the part about "optional" argument. I think that clears things up for me.
@frewsxcv frewsxcv added the C-assigned label Dec 5, 2015
frewsxcv added a commit to frewsxcv/servo that referenced this issue Dec 5, 2015
@frewsxcv
Treat 'undefined' passed to an optional argument as "missing"
de77653 
Fixes servo#8813
frewsxcv added a commit to frewsxcv/servo that referenced this issue Dec 5, 2015
@frewsxcv
Treat 'undefined' passed to an optional argument as "missing"
8432155 
Fixes servo#8813
@frewsxcv frewsxcv mentioned this issue Dec 5, 2015
Closed
@KiChjang KiChjang mentioned this issue Dec 6, 2015
Merged
bors-servo added a commit that referenced this issue Dec 6, 2015
@bors-servo
Auto merge of #8854 - KiChjang:undefined-as-missing, r=<try>
c3c2a1a 
Treat undefined arguments in JS as missing

@frewsxcv please don't hurt me for this.

I've added an AND condition to check whether the value being passed is undefined while checking whehther the argument exists at all. Essentially, this is now treating undefined arguments the same as missing arguments.

Fixes #8813.
Fixes #6558.
bors-servo added a commit that referenced this issue Dec 6, 2015
@bors-servo
Auto merge of #8854 - KiChjang:undefined-as-missing, r=<try>
252e7fb 
Treat 'undefined' passed to optional JS arguments as missing

@frewsxcv please don't hurt me for this.

I've added an AND condition to check whether the value being passed is undefined while checking whether the argument exists at all. Essentially, this is now treating undefined arguments the same as missing arguments.

Fixes #8813.
Fixes #6558.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8854)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Dec 6, 2015
@bors-servo
Auto merge of #8849 - frewsxcv:passing-undefined-into-optional-argume…
b7d5928 
…nt, r=<try>

Treat 'undefined' passed to an optional argument as "missing"

Fixes #8813

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8849)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Dec 6, 2015
@bors-servo
Auto merge of #8854 - KiChjang:undefined-as-missing, r=<try>
2fffc00 
Treat 'undefined' passed to optional JS arguments as missing

@frewsxcv please don't hurt me for this.

I've added an AND condition to check whether the value being passed is undefined while checking whether the argument exists at all. Essentially, this is now treating undefined arguments the same as missing arguments.

Fixes #8813.
Fixes #6558.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8854)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Dec 7, 2015
@bors-servo
Auto merge of #8854 - KiChjang:undefined-as-missing, r=<try>
878416f 
Treat 'undefined' passed to optional JS arguments as missing

@frewsxcv please don't hurt me for this.

I've added an AND condition to check whether the value being passed is undefined while checking whether the argument exists at all. Essentially, this is now treating undefined arguments the same as missing arguments.

Fixes #8813.
Fixes #6558.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8854)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Dec 11, 2015
@bors-servo
Auto merge of #8854 - KiChjang:undefined-as-missing, r=<try>
e34cdc1 
Treat 'undefined' passed to optional JS arguments as missing

@frewsxcv please don't hurt me for this.

I've added an AND condition to check whether the value being passed is undefined while checking whether the argument exists at all. Essentially, this is now treating undefined arguments the same as missing arguments.

Fixes #8813.
Fixes #6558.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8854)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Dec 11, 2015
@bors-servo
Auto merge of #8854 - KiChjang:undefined-as-missing, r=<try>
a04a394 
Treat 'undefined' passed to optional JS arguments as missing

@frewsxcv please don't hurt me for this.

I've added an AND condition to check whether the value being passed is undefined while checking whether the argument exists at all. Essentially, this is now treating undefined arguments the same as missing arguments.

Fixes #8813.
Fixes #6558.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8854)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Dec 11, 2015
@bors-servo
Auto merge of #8854 - KiChjang:undefined-as-missing, r=<try>
304fda1 
Treat 'undefined' passed to optional JS arguments as missing

@frewsxcv please don't hurt me for this.

I've added an AND condition to check whether the value being passed is undefined while checking whether the argument exists at all. Essentially, this is now treating undefined arguments the same as missing arguments.

Fixes #8813.
Fixes #6558.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8854)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Dec 11, 2015
@bors-servo
Auto merge of #8854 - KiChjang:undefined-as-missing, r=<try>
2660b9f 
Treat 'undefined' passed to optional JS arguments as missing

@frewsxcv please don't hurt me for this.

I've added an AND condition to check whether the value being passed is undefined while checking whether the argument exists at all. Essentially, this is now treating undefined arguments the same as missing arguments.

Fixes #8813.
Fixes #6558.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8854)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Dec 13, 2015
@bors-servo
Auto merge of #8854 - KiChjang:undefined-as-missing, r=frewsxcv
Loading status checks…
8bab1cd 
Treat 'undefined' passed to optional JS arguments as missing

@frewsxcv please don't hurt me for this.

I've added an AND condition to check whether the value being passed is undefined while checking whether the argument exists at all. Essentially, this is now treating undefined arguments the same as missing arguments.

Fixes #8813.
Fixes #6558.

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

@frewsxcv frewsxcv

Labels
A-content/bindings C-assigned
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
@Ms2ger @frewsxcv @eefriedman
You can’t perform that action at this time.