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

fix(material/checkbox): clear name from host node #15368

Closed
wants to merge 1 commit into from

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Mar 2, 2019

Currently we forward the name attribute from the host node to the underlying input, however we leave the name on the host node intact. This can throw off functions like document.getElementsByName or the By.name Protractor selector.

@crisbeto crisbeto added the target: patch This PR is targeted for the next patch release label Mar 2, 2019
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Mar 2, 2019
josephperrott
josephperrott previously approved these changes Mar 4, 2019
Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM

@josephperrott josephperrott added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Mar 4, 2019
crisbeto added a commit to crisbeto/material2 that referenced this pull request Mar 8, 2019
Along the same lines as angular#15368. Clears the `name` from the host node of the radio button and radio group, because they end up being forwarded to the underlying `input` and can cause double results when using something like `getElementsByName`.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Mar 16, 2019
Along the same lines as angular#15422 and angular#15368. Clears the static `name` attribute from the slide toggle's host node in order to prevent both the underlying input and the host from showing up in `document.getElementsByName` or `By.name`.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Mar 23, 2019
Along the same lines as angular#15422 and angular#15368. Clears the static `name` attribute from the slide toggle's host node in order to prevent both the underlying input and the host from showing up in `document.getElementsByName` or `By.name`.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Mar 31, 2019
Along the same lines as angular#15368. Since we forward the `name` to the underlying native inside a button toggle, otherwise the results from `document.getElementsByName` or Protractor's `By.name` might be thrown off.
@mmalerba mmalerba added aaa and removed aaa labels Apr 25, 2019
crisbeto added a commit to crisbeto/material2 that referenced this pull request May 4, 2019
Along the same lines as angular#15422 and angular#15368. Clears the static `name` attribute from the slide toggle's host node in order to prevent both the underlying input and the host from showing up in `document.getElementsByName` or `By.name`.
crisbeto added a commit to crisbeto/material2 that referenced this pull request May 13, 2019
Along the same lines as angular#15368. Since we forward the `name` to the underlying native inside a button toggle, otherwise the results from `document.getElementsByName` or Protractor's `By.name` might be thrown off.
jelbourn pushed a commit that referenced this pull request May 13, 2019
Along the same lines as #15368. Since we forward the `name` to the underlying native inside a button toggle, otherwise the results from `document.getElementsByName` or Protractor's `By.name` might be thrown off.
RudolfFrederiksen pushed a commit to RudolfFrederiksen/material2 that referenced this pull request Jun 21, 2019
Along the same lines as angular#15368. Since we forward the `name` to the underlying native inside a button toggle, otherwise the results from `document.getElementsByName` or Protractor's `By.name` might be thrown off.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Jul 1, 2019
Along the same lines as angular#15368. Clears the `name` from the host node of the radio button and radio group, because they end up being forwarded to the underlying `input` and can cause double results when using something like `getElementsByName`.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Jul 21, 2019
Along the same lines as angular#15422 and angular#15368. Clears the static `name` attribute from the slide toggle's host node in order to prevent both the underlying input and the host from showing up in `document.getElementsByName` or `By.name`.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Oct 1, 2019
Along the same lines as angular#15368. Clears the `name` from the host node of the radio button and radio group, because they end up being forwarded to the underlying `input` and can cause double results when using something like `getElementsByName`.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Nov 29, 2019
Along the same lines as angular#15422 and angular#15368. Clears the static `name` attribute from the slide toggle's host node in order to prevent both the underlying input and the host from showing up in `document.getElementsByName` or `By.name`.
crisbeto added a commit to crisbeto/material2 that referenced this pull request May 2, 2020
Along the same lines as angular#15368. Clears the `name` from the host node of the radio button and radio group, because they end up being forwarded to the underlying `input` and can cause double results when using something like `getElementsByName`.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Jun 7, 2020
Along the same lines as angular#15368. Clears the `name` from the host node of the radio button and radio group, because they end up being forwarded to the underlying `input` and can cause double results when using something like `getElementsByName`.
@mmalerba mmalerba removed the lgtm label Jul 31, 2020
crisbeto added a commit to crisbeto/material2 that referenced this pull request Jan 10, 2021
Along the same lines as angular#15422 and angular#15368. Clears the static `name` attribute from the slide
toggle's host node in order to prevent both the underlying input and the host from showing
up in `document.getElementsByName` or `By.name`.
@devversion devversion removed their request for review August 18, 2021 12:55
@andrewseguin andrewseguin added needs rebase and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels Dec 29, 2021
crisbeto added a commit to crisbeto/material2 that referenced this pull request Mar 7, 2022
Along the same lines as angular#15422 and angular#15368. Clears the static `name` attribute from the slide
toggle's host node in order to prevent both the underlying input and the host from showing
up in `document.getElementsByName` or `By.name`.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Mar 7, 2022
Along the same lines as angular#15368. Clears the `name` from the host node of the radio button and radio group, because they end up being forwarded to the underlying `input` and can cause double results when using something like `getElementsByName`.
crisbeto added a commit that referenced this pull request Mar 9, 2022
Along the same lines as #15422 and #15368. Clears the static `name` attribute from the slide
toggle's host node in order to prevent both the underlying input and the host from showing
up in `document.getElementsByName` or `By.name`.

(cherry picked from commit 5db1df0)
crisbeto added a commit that referenced this pull request Mar 9, 2022
Along the same lines as #15422 and #15368. Clears the static `name` attribute from the slide
toggle's host node in order to prevent both the underlying input and the host from showing
up in `document.getElementsByName` or `By.name`.
forsti0506 pushed a commit to forsti0506/components that referenced this pull request Apr 3, 2022
Along the same lines as angular#15422 and angular#15368. Clears the static `name` attribute from the slide
toggle's host node in order to prevent both the underlying input and the host from showing
up in `document.getElementsByName` or `By.name`.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Aug 23, 2022
Along the same lines as angular#15368. Clears the `name` from the host node of the radio button and radio group, because they end up being forwarded to the underlying `input` and can cause double results when using something like `getElementsByName`.
@josephperrott josephperrott added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed needs rebase labels Nov 16, 2022
Currently we forward the name attribute from the host node to the underlying input, however we leave the name on the host node intact. This can throw off functions like `document.getElementsByName` or the `By.name` Protractor selector.
@crisbeto crisbeto changed the title fix(checkbox): clear name from host node fix(material/checkbox): clear name from host node Feb 28, 2024
@crisbeto crisbeto removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Feb 28, 2024
@crisbeto
Copy link
Member Author

Looks like this is really breaking internally. I'll close it for now since we haven't gotten any issue reports about it.

@crisbeto crisbeto closed this Feb 28, 2024
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Mar 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants