PolymerElements / paper-input

A Material Design text field
https://www.webcomponents.org/element/PolymerElements/paper-input
130 stars 159 forks source link

Error: non-unique id #input #600

Closed PaulHMason closed 6 years ago

PaulHMason commented 6 years ago

Description

Chrome (v63.0.3239.84) is suddenly reporting the following errors for paper-input (of type password) and gold-email-input:

[DOM] Found x elements with non-unique id #input: (More info: https://goo.gl/9p2vKq) `

`

This is for the latest v1.x (not v2).

It may be due to this? https://groups.google.com/a/chromium.org/forum/#!topic/blink-reviews-html/xQhBySgZRZ4

Steps to reproduce

  1. Put a couple of paper-input elements (of type password) in the page.
  2. Open the page in Chrome.
  3. The errors will be reported in the console.

Possibly change it to use a class internally, rather than an id?

Browsers Affected

notwaldorf commented 6 years ago

I think this is because you're running shady DOM, where there's no actual DOM encapsulation.

The ID is used in the code, so we can't just change it to a class. Also, this is the whole point of shadow DOM, and this is just a limitation of the polyfill, so I don't think there's much we can do about this

PaulHMason commented 6 years ago

It is a Shady DOM issue, but it's odd that Chrome is reporting it as an error (it should be a warning, according to the documentation).

I played around with it a bit - the following changes work (unit tests all pass - still need to check everything else).

paper-input

Remove id and add class="internal__input"

paper-input-behavior

get inputElement() {
    //return this.$.input;
    return this.$$(".internal__input");
}

paper-input-container

_inputSelector: {
    type: String,
    value: 'input,textarea,.paper-input-input,.internal__input'
}

paper-textarea

Same thing as paper-input - just need to change the internal selector to a class selector, rather than use $.input.

bstmedia commented 6 years ago

I'm embarrassed by my own work.. but you can also consider to change the field type to something else then "password" and apply " -webkit-text-security: disc" with CSS to the field. This will clear the chrome error and keep the password input behavior.

However.. i'm not suggesting anyone do this.. and would hope to see a more appropriate solution posted here.

nochev commented 6 years ago

What about to get the input by tag name:

    /**
     * Returns a reference to the input element.
     */
    get inputElement() {
      return this.querySelector('input');
    },
    ...

Even with this, we will still need the id attribute for label's for attribute. I suppose it can be generated randomly:

  Polymer({
    is: 'paper-input',
    properties: {
        inputId: {
            type: 'String',
            value: function() {
                return 'input_' + Math.random().toString(36).substring(2);
            }
        }
    },

    behaviors: [
      Polymer.IronFormElementBehavior,
      Polymer.PaperInputBehavior
    ]
  });

and respectively in html:

      <label hidden$="[[!label]]" aria-hidden="true" for="[[inputId]]">[[label]]</label>

      <input is="iron-input" id="[[inputId]]"
        aria-labelledby$="[[_ariaLabelledBy]]"

        ...

What do you think of such a solution?

contis2908 commented 6 years ago

The same error occurs If I am using the standard html input field and apply type="password" to it. I don't really understand why the input is="iron-input" has duplicate id's if I not even use it. On top of all this error get's called 22 although I am using it only ones

gitact commented 6 years ago

Hello! Tried some solutions but it seems they are only partial and limited solutions for input problems and derived polymers :-(

You know we surely cannot go to Production environment with our webapp logging twinklinkg Errors on Chrome 63 console, like lights on a Christmas tree :-P

Any other brilliant (and final) ideas as Christmas gift from mythical Monica @notwaldorf and fascinating @robdodson (or vice versa) or other Polymer Team guys ? Thanks and greetings!

fgirardey commented 6 years ago

You know we surely cannot go to Production environment with our webapp logging twinklinkg Errors on Chrome 63 console, like lights on a Christmas tree :-P

How is that block for your deployment process? This error log is just a log, it doesn't break anything in any app.

Pietfro commented 6 years ago

How is that block for your deployment process? This error log is just a log, it doens't break anything in any app.

IMO, when you are about to deploy an application that throws a bunch of errors to a client's setup that is an issue.

I like @nochev solution and probably will go with similar approach.

fgirardey commented 6 years ago

IMO, when you are about to deploy an application that throws a bunch of errors to a client's setup that is an issue.

It is not new that having duplicate ids in a document is not according to the HTML spec, the only new thing here is that Chrome log (and does not throw it) an error that where there before Chrome 63…

nochev commented 6 years ago

@fgirardey you are absolutely right, but If there is solution to avoid these annoying logs, it is better to be resolved. Also there is no guarantee that Chrome can add some other limitations for duplicate ids in the future.

notwaldorf commented 6 years ago

Ooops sorry, didn't mean to close!

Did a bit of sherlocking: this error only comes up if you have an <input type="password" id="foo"> and another input of any type with the same ID. I originally thought this problem applied to all duplicate IDs, but it seems very input related.

gitact commented 6 years ago

Sorry Monica @notwaldorf , but maybe your answer wasn't so clear/complete, at least this time ;-P

It happens in other situations of multiple "password paper-input"s as well. Please see below the classic example for a change password card

            <paper-input id="newPassword" type="password"></paper-input>
            <paper-input id="confirmPassword" type="password"></paper-input>

We'll get an error "[DOM] Found 2 elements with non-unique id #input" and even if we Polymer guys know that Paper-input implementation includes a classic html input named "input", we cannot govern this.

Does it mean we have to go to Production with multiple Error on the console? (that our clients are yet complaining of).

Any other ideas from @robdodson or other Polymer Team guys ?

notwaldorf commented 6 years ago

Yes I understand that. For the record, I am a core member of the Polymer team, and I own this element, so you don't need to dial in other people for help.

What I had explained is the underlying problem, not the symptom: paper-input contains an input (and it passes down the type to it); because you're using shady dom, that input isn't encapsulated in a shadow DOM, thus leading to the problem I mentioned.

There is already a PR opened where we are trying to fix this. If you can't wait for that, then you can just turn on shadow DOM on Chrome (which you should be doing anyway). You can do this by using webcomponents-loader instead of webcomponents-lite in V1: https://github.com/webcomponents/webcomponentsjs#webcomponents-loaderjs

gitact commented 6 years ago

Yes we perfectly know (if you remember the prevoious post Christmas gift from mythical Monica @notwaldorf and fascinating @robdodson (or vice versa) or other Polymer Team guys ) Trying using Shadow Dom wasn't sufficient (and you know it is riskful for some browser) because of the behaviors of other external web-components on GitHub, we adopted. We obviously appreciate your work, but hope you understand or point of view...

Thanks as always Monica and a wonderful 2018 ;-)

notwaldorf commented 6 years ago

This is about using shadow DOM only on Chrome, not other browsers. webcomponent-loader conditionally loads the polyfills based on what browser you're on, so on browsers that already have a native shadow DOM implementation, like Chrome, you would just get that.

In any case, if you don't want to do that, then please wait for the open PR to get merged (#602)

notwaldorf commented 6 years ago

This has been fixed and released for Polymer 1 as 1.2.0. (Polymer 2 and hybrid should hopefully be released today)

Boscop commented 6 years ago

Why do I get this error suddenly in chrome when my polymer app was working without errors before?

[DOM] Found 9 elements with non-unique id #searchBox
...
[DOM] Found 56 elements with non-unique id #input
...
gitact commented 6 years ago

Hello. you could read above that @notwaldorf closed this in #609 4 days ago

In detail, it depends on new Chrome 63 version and the fix will resolve the best part of "[DOM] Found XX elements with non-unique id #input" when depending on official "paper-input". Not if depending on other un-official cloned polymers (some examples exist on GitHub), nor if depending on #searchBox elements or similar belonging to other custom webcomponents: in this case you could ask a question or better a Pull Request to GitHub contributors of your favourite but imperfect component (paper-searchbox?)

Hoping can help ;-)

Boscop commented 6 years ago

Thanks, but I am already on chrome 63 and the issue is still happening (Version 63.0.3239.132). Also, do I have to update paper-input? What's the best way to update paper-input, I'm stuck with polymer 1.7 for this app.

Also, what actually changed that this error only occurs now and not before?

notwaldorf commented 6 years ago

What changed is that Chrome introduced new auditing for inputs, which logs this as an error.

This fix has also been released in version 1.2.0 for Polymer 1 apps. You can just re-run bower install to pick it up, or pin the version at that in your bower.json

dman777 commented 6 years ago

Note: I just realized that I am probably getting this message from iron-elements and not paper-input. Sorry about that.

felixzapata commented 6 years ago

hi, we still have the problems with duplicated ids under Chrome 64.

The console throws [DOM] Found 4 elements with non-unique id #:

captura de pantalla 2018-02-19 a las 11 45 42

We are using Shady DOM with:

In the previous screenshot I can't find any duplicate id. Maybe the browser detect them before the id being created.

WTober commented 6 years ago

I have updatet to paper-input 1.2.1 and console messages "elements with non-unique id #input:" are no longer displayed.

How can I fix this? nonunique_id

firdousy commented 6 years ago

Using v 2.2.3, I noticed The id on the input is binded using attribute bindings ($). i.e <input is="iron-input" id$="[[_inputId]]" where it should be without the dollar sign. Removing the dollar sign removes these errors in the console. So it should be: <input is="iron-input" id="[[_inputId]]"