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

won't login into zoho.com because of missing named getter on HTMLFormElement #16479

Closed
plasmoidian opened this issue Apr 16, 2017 · 22 comments
Closed

Comments

@plasmoidian
Copy link

@plasmoidian plasmoidian commented Apr 16, 2017

Servo reports an error in the authentication. Sometimes it freezes afterwards. Windows build.

@jdm
Copy link
Member

@jdm jdm commented Apr 17, 2017

The console shows

ERROR:script::dom::bindings::error: Error at https://accounts.zoho.com/login?servicename=ZohoHome&hidesignup=true&&hide_secure=true&css=https://www.zoho.com/css/prd-sign.css:384:21 frm.lid is undefined

This is part of the code that is executed during the form submission; we are missing the named getter on HTMLFormElement which allows referencing form elements by their id.

@jdm jdm added A-content/dom and removed A-network labels Apr 17, 2017
@jdm jdm changed the title won't login into zoho.com won't login into zoho.com because of missing named getter on HTMLFormElement Apr 17, 2017
@jdm
Copy link
Member

@jdm jdm commented Apr 17, 2017

Code: components/script/dom/htmlformelement.rs, components/script/dom/webidls/HTMLFormElement.webidl
Tests: ./mach test-wpt tests/wpt/web-platform-tests/html/semantics/forms/the-form-element/

@ghost
Copy link

@ghost ghost commented May 5, 2017

I'd like to try to implement this. Not very experienced, but I've got the summer cleared up.

@jdm
Copy link
Member

@jdm jdm commented May 5, 2017

Please do! Ask questions about anything that's unclear!

@jdm jdm added the C-assigned label May 5, 2017
@ghost
Copy link

@ghost ghost commented May 14, 2017

I can't seem to find past names map in the htmlformelement.rs file. Do you know where I might find it? The following lines seem to be what I want but I'm not entirely sure since it implements only to step 2.2 of this spec.

@jdm
Copy link
Member

@jdm jdm commented May 14, 2017

We do not implement the past names map yet. Adding support for it is required for the named getter.

@jdm
Copy link
Member

@jdm jdm commented May 14, 2017

If you have questions about how to implement the past names map, I am happy to answer them :)

@ghost
Copy link

@ghost ghost commented May 19, 2017

@jdm I'm not quite sure how to implement this. I understand from the documentation that the map is member variable in htmlformelement and that the bindings will handle the lifetimes of the object and not its implementation. So I figure that all the info that I'd need sits on the rust side, but I don't know how I'm to keep track of the changes made on each of the children of htmlformelement. Should I modify htmlelement to alert it's parent on a name change or is there something I'm missing?

@ghost
Copy link

@ghost ghost commented May 21, 2017

Think I may have found what I'm looking for with the attribute list within elements. Would love your input if I'm still way off base.

@jdm
Copy link
Member

@jdm jdm commented May 22, 2017

The more I read the specification's description of the past names map, the weirder it becomes. It appears that you do not need to track changes to the children of a form element (as sensible as that might seem!). Instead, the specification describes behaviour that allows this script to show undefined for the input element's original id.

Let's look at all of the references to the "past names map" in the specification (click on the bold text to see the list for yourself):

  • the definition
  • including all of the entries from the map in the list of supported property names (this algorithm corresponds to the SupportedPropertyNames method that is required when a named getter operation is present in a WebIDL interface)
  • determining which object to return for a given supported property name (this would be in the implementation of the NamedGetter method that is required when a named getter operation is present in a WebIDL interface)
  • adding an entry to the map as part of the previous algorithm
  • removing an entry from the map if it no longer belongs to the given form element

All of these are lazy activities - the map only ever is modified during the process of returning the list of supported property names. This means that this algorithm returns the list of names of elements that currently belong to this form, and the list of names that were known last time the list was created. Am I making sense? In short, I would be surprised to see many (if any) changes outside of htmlformelement.rs, since HTMLFormElement has a controls member which can give us the list of controls belonging to that form element.

@ghost
Copy link

@ghost ghost commented May 22, 2017

Thanks so much! This makes a lot more sense.

@ghost
Copy link

@ghost ghost commented Jun 5, 2017

Simple question. In step two from here it mentions the image object. For what I've written I've assumed it means an image button, as I think a regular image wouldn't be of much use for form submission. So I'm wondering if this is correct or if I should swap it to use the html image?

@jdm
Copy link
Member

@jdm jdm commented Jun 5, 2017

I agree that it's confusing! Given that the previous step is very explicit about image buttons, I recommend following the instructions as written. I'll see if I can figure out why they care about img elements in that algorithm.

@ghost
Copy link

@ghost ghost commented Jun 5, 2017

I went looking through standard and found that usemap, a member variable of img, is considered palpable content. So I'm guessing a form might use it to choose between options.

@jdm
Copy link
Member

@jdm jdm commented Jul 5, 2017

@WholeGrainGoats Are you still working on this?

@ghost
Copy link

@ghost ghost commented Jul 6, 2017

Yes, I started debugging this Monday. Sorry for taking so long. I do have a question, though. Is this ./mach test-wpt tests/wpt/web-platform-tests/html/semantics/forms/the-form-element/ enough to test fully or should I add some more test cases to it? I say that because I ran it at the beginning, before I uncommented the corresponding WebIDL and had written the code, but the tests all succeeded.

@jdm
Copy link
Member

@jdm jdm commented Jul 6, 2017

Yes, since we have lists of expected results like https://dxr.mozilla.org/servo/source/tests/wpt/metadata/html/semantics/forms/the-form-element/form-nameditem.html.ini which currently expect failures for all tests related to the named getter. Those should end up showing "PASS expected FAIL" once your changes take effect.

@jdm
Copy link
Member

@jdm jdm commented Sep 5, 2017

@WholeGrainGoats Are you still working on this?

@ghost
Copy link

@ghost ghost commented Oct 16, 2017

@jdm jdm removed the C-assigned label Oct 16, 2017
@paavininanda
Copy link
Contributor

@paavininanda paavininanda commented Mar 11, 2018

Hi, @jdm! I would like to work on this issue.

@jaymodi98 jaymodi98 mentioned this issue Dec 3, 2019
3 of 3 tasks complete
@pshaughn
Copy link
Member

@pshaughn pshaughn commented Dec 6, 2019

Lack of a named getter makes WPT html/semantics/forms/form-submission-0/constructing-form-data-set.html fail (slightly confusingly, since the button is named "submit" and expected to shadow the built-in .submit without a comment explaining what's going on)

bors-servo added a commit that referenced this issue Dec 7, 2019
Named form getter

This PR contains changes related to adding named getter in Servo for getting the list of all meaningful property names for a HTMLFormElement object, and getting the value of a specific property name. The following changes have been made:

* uncomment the [named getter](https://github.com/servo/servo/blob/f63b404e0cbf30380c4043700861110d06e548bb/components/script/dom/webidls/HTMLFormElement.webidl#L30) from HTMLFormElement.webidl
* add the missing `NamedGetter` and `SupportedPropertyNames` methods to [HTMLFormElement](https://github.com/servo/servo/blob/f63b404e0cbf30380c4043700861110d06e548bb/components/script/dom/htmlformelement.rs#L113)
* implement `SupportedPropertyNames` according to [the specification](https://html.spec.whatwg.org/multipage/forms.html#the-form-element:supported-property-names):
  * create an enum to represent the `id`, `name`, and `past` states for the sourced names
  * create a vector of `(SourcedName, DomRoot<HTMLElement>)` by iterating over `self.controls` and checking the element type and calling methods like `HTMLElement::is_listed_element`
---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #16479 (GitHub issue number if applicable)
bors-servo added a commit that referenced this issue Dec 8, 2019
Named form getter

This PR contains changes related to adding named getter in Servo for getting the list of all meaningful property names for a HTMLFormElement object, and getting the value of a specific property name. The following changes have been made:

* uncomment the [named getter](https://github.com/servo/servo/blob/f63b404e0cbf30380c4043700861110d06e548bb/components/script/dom/webidls/HTMLFormElement.webidl#L30) from HTMLFormElement.webidl
* add the missing `NamedGetter` and `SupportedPropertyNames` methods to [HTMLFormElement](https://github.com/servo/servo/blob/f63b404e0cbf30380c4043700861110d06e548bb/components/script/dom/htmlformelement.rs#L113)
* implement `SupportedPropertyNames` according to [the specification](https://html.spec.whatwg.org/multipage/forms.html#the-form-element:supported-property-names):
  * create an enum to represent the `id`, `name`, and `past` states for the sourced names
  * create a vector of `(SourcedName, DomRoot<HTMLElement>)` by iterating over `self.controls` and checking the element type and calling methods like `HTMLElement::is_listed_element`
---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #16479 (GitHub issue number if applicable)
bors-servo added a commit that referenced this issue Dec 16, 2019
Named form getter

This PR contains changes related to adding named getter in Servo for getting the list of all meaningful property names for a HTMLFormElement object, and getting the value of a specific property name. The following changes have been made:

* uncomment the [named getter](https://github.com/servo/servo/blob/f63b404e0cbf30380c4043700861110d06e548bb/components/script/dom/webidls/HTMLFormElement.webidl#L30) from HTMLFormElement.webidl
* add the missing `NamedGetter` and `SupportedPropertyNames` methods to [HTMLFormElement](https://github.com/servo/servo/blob/f63b404e0cbf30380c4043700861110d06e548bb/components/script/dom/htmlformelement.rs#L113)
* implement `SupportedPropertyNames` according to [the specification](https://html.spec.whatwg.org/multipage/forms.html#the-form-element:supported-property-names):
  * create an enum to represent the `id`, `name`, and `past` states for the sourced names
  * create a vector of `(SourcedName, DomRoot<HTMLElement>)` by iterating over `self.controls` and checking the element type and calling methods like `HTMLElement::is_listed_element`
---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #16479 (GitHub issue number if applicable)
bors-servo added a commit that referenced this issue Dec 16, 2019
Named form getter

This PR contains changes related to adding named getter in Servo for getting the list of all meaningful property names for a HTMLFormElement object, and getting the value of a specific property name. The following changes have been made:

* uncomment the [named getter](https://github.com/servo/servo/blob/f63b404e0cbf30380c4043700861110d06e548bb/components/script/dom/webidls/HTMLFormElement.webidl#L30) from HTMLFormElement.webidl
* add the missing `NamedGetter` and `SupportedPropertyNames` methods to [HTMLFormElement](https://github.com/servo/servo/blob/f63b404e0cbf30380c4043700861110d06e548bb/components/script/dom/htmlformelement.rs#L113)
* implement `SupportedPropertyNames` according to [the specification](https://html.spec.whatwg.org/multipage/forms.html#the-form-element:supported-property-names):
  * create an enum to represent the `id`, `name`, and `past` states for the sourced names
  * create a vector of `(SourcedName, DomRoot<HTMLElement>)` by iterating over `self.controls` and checking the element type and calling methods like `HTMLElement::is_listed_element`
---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #16479 (GitHub issue number if applicable)
bors-servo added a commit that referenced this issue Dec 16, 2019
Named form getter

This PR contains changes related to adding named getter in Servo for getting the list of all meaningful property names for a HTMLFormElement object, and getting the value of a specific property name. The following changes have been made:

* uncomment the [named getter](https://github.com/servo/servo/blob/f63b404e0cbf30380c4043700861110d06e548bb/components/script/dom/webidls/HTMLFormElement.webidl#L30) from HTMLFormElement.webidl
* add the missing `NamedGetter` and `SupportedPropertyNames` methods to [HTMLFormElement](https://github.com/servo/servo/blob/f63b404e0cbf30380c4043700861110d06e548bb/components/script/dom/htmlformelement.rs#L113)
* implement `SupportedPropertyNames` according to [the specification](https://html.spec.whatwg.org/multipage/forms.html#the-form-element:supported-property-names):
  * create an enum to represent the `id`, `name`, and `past` states for the sourced names
  * create a vector of `(SourcedName, DomRoot<HTMLElement>)` by iterating over `self.controls` and checking the element type and calling methods like `HTMLElement::is_listed_element`
---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #16479 (GitHub issue number if applicable)
bors-servo added a commit that referenced this issue Dec 16, 2019
Named form getter

This PR contains changes related to adding named getter in Servo for getting the list of all meaningful property names for a HTMLFormElement object, and getting the value of a specific property name. The following changes have been made:

* uncomment the [named getter](https://github.com/servo/servo/blob/f63b404e0cbf30380c4043700861110d06e548bb/components/script/dom/webidls/HTMLFormElement.webidl#L30) from HTMLFormElement.webidl
* add the missing `NamedGetter` and `SupportedPropertyNames` methods to [HTMLFormElement](https://github.com/servo/servo/blob/f63b404e0cbf30380c4043700861110d06e548bb/components/script/dom/htmlformelement.rs#L113)
* implement `SupportedPropertyNames` according to [the specification](https://html.spec.whatwg.org/multipage/forms.html#the-form-element:supported-property-names):
  * create an enum to represent the `id`, `name`, and `past` states for the sourced names
  * create a vector of `(SourcedName, DomRoot<HTMLElement>)` by iterating over `self.controls` and checking the element type and calling methods like `HTMLElement::is_listed_element`
---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #16479 (GitHub issue number if applicable)
bors-servo added a commit that referenced this issue Dec 16, 2019
Named form getter

This PR contains changes related to adding named getter in Servo for getting the list of all meaningful property names for a HTMLFormElement object, and getting the value of a specific property name. The following changes have been made:

* uncomment the [named getter](https://github.com/servo/servo/blob/f63b404e0cbf30380c4043700861110d06e548bb/components/script/dom/webidls/HTMLFormElement.webidl#L30) from HTMLFormElement.webidl
* add the missing `NamedGetter` and `SupportedPropertyNames` methods to [HTMLFormElement](https://github.com/servo/servo/blob/f63b404e0cbf30380c4043700861110d06e548bb/components/script/dom/htmlformelement.rs#L113)
* implement `SupportedPropertyNames` according to [the specification](https://html.spec.whatwg.org/multipage/forms.html#the-form-element:supported-property-names):
  * create an enum to represent the `id`, `name`, and `past` states for the sourced names
  * create a vector of `(SourcedName, DomRoot<HTMLElement>)` by iterating over `self.controls` and checking the element type and calling methods like `HTMLElement::is_listed_element`
---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #16479 (GitHub issue number if applicable)
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.

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