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

Prevent duplicate ID's in polyfill #295

Closed
stefanjudis opened this issue Sep 25, 2013 · 21 comments
Closed

Prevent duplicate ID's in polyfill #295

stefanjudis opened this issue Sep 25, 2013 · 21 comments

Comments

@stefanjudis
Copy link

When using id's in a polymer component and using the new element multiple times, the fallback solution will render the same html multiple times, which leads to multiple id's inside of the DOM, invalid html and maybe misbehaviour.

For example:

<polymer-element name="sj-checkbox" attributes="size">
  <template>
    <style>
    /* all my styles here */
    </style>
    <div class="switch {{size}}">
      <div class="switch__circle">
          <input id="switcher" class="switch__input" type="checkbox" on-change="changeHandler" checked="checked">
          <div class="switch__innerCircles"></div>
          <label class="switch__label" for="switcher">Switch me!!!</label>
        </div>
      </div>
    </div>
  </template>
  <script>
    Polymer( 'sj-checkbox', {
      size: 'medium',
      changeHandler: function( event ) {
        this.fire(
          'change',
          {
            id: event.target.id
          }
        );
      }
    } );
  </script>
</polymer-element>

will become:

<sj-checkbox size="big" fallbackid="checkboxBigComponent">
  <div class="switch big">
    <div class="switch__circle">
      <input id="switcher" class="switch__input" type="checkbox" on-change="changeHandler" checked="checked">
      <div class="switch__innerCircles"></div>
      <label class="switch__label" for="switcher">Switch me!!!</label>
    </div>
  </div>
</sj-checkbox>

Unfortunately the component is not reusable in this case.

@sjmiles
Copy link
Contributor

sjmiles commented Sep 25, 2013

The duplicate id's are inside the component's Shadow DOM which is a protected scope and therefore free to re-use ids.

Your "will become" code represents the render tree, which is not the DOM you are to interact with. You can see that DOM if you use system inspectors on browsers that do not support Shadow DOM natively, but the Shadow DOM polyfill should present a DOM API that is correctly scoped.

Try your example on Canary to see how it works with native Shadow DOM.

@stefanjudis
Copy link
Author

Jap, but in case the browser doesn't support Shadow DOM I end up with missbehaviour.

You can check it here in the third example: http://stefanjudis.github.io/webComponents-tutorial/

Canary works fine as expected and other browsers unfortunately not.

@sjmiles
Copy link
Contributor

sjmiles commented Sep 25, 2013

Yes, in this case you are relying on DOM-level id matching via the for attribute on <label>.

The Shadow DOM polyfill does it's best to provide the proper DOM API to user-level code, but we cannot trick the actual user agent, so I don't believe we can ever make this work with ShadowDOM polyfill.

This is unfortunate wrt confounding user's expectations, but as a practical matter it is easy to work around in a custom element scenario.

This is important information to add to our FAQ.

@ebidel

@sjmiles
Copy link
Contributor

sjmiles commented Sep 25, 2013

Fwiw, ironically, you have two nodes with id="switchesComponents" at top-level of your document, which is causing some confusion.

@stefanjudis
Copy link
Author

😄 Ha, sorry. Quick copy and pasting for showing it.

Maybe I don't get the complexity of the whole polyfill, but couldn't it scan for id's inside of the template and add a random hash to the id's before appending it to the DOM for not supporting browsers?

Workaround worked fine for me, so not a big deal. ;)

Thx.

@sjmiles
Copy link
Contributor

sjmiles commented Sep 25, 2013

Thanks for the idea. I think in general it would be a bad idea to scramble id's because the user could be relying on them in various ways.

We could maybe add a helper method that does this rewriting, opt-in, if somebody wants to use label for.

We probably need to catalog the scenarios where DOM uses id's internally like that.

@stefanjudis
Copy link
Author

👍 Helper sounds good.

@ebidel
Copy link
Contributor

ebidel commented Sep 26, 2013

What's the best way to describe this?

@stefanjudis
Copy link
Author

Sorry, I don't understand. What do you mean by 'this'? The helper functionality?

@ebidel
Copy link
Contributor

ebidel commented Sep 26, 2013

I'm asking what needs to be mentioned in the FAQ? Can't use DOM-level id matching (<label for>) in the shadow dom polyfill?

@stefanjudis
Copy link
Author

Unfortunately it's not only label for. Invalid markup with duplicate id's is also a result, which should be avoided.

I would maybe think of something like:

I want to use polymer mainly for not supporting browsers because I love the princible of web components. Is there anything I should pay attention to?

If you want to reuse a web component multiple times inside of the same document, you should avoid the usage of id's inside of your component. Unfortunately the depending polyfill has no solution for id handling yet, which means that in that case multiple DOM nodes with the same id will appended to the DOM. This could lead to invalid markup and in some cases misbehaviour (like label for relation).

@stefanjudis
Copy link
Author

A more general approach with a warning of careful usage with id's is a nice approach, i guess.

@sjmiles
Copy link
Contributor

sjmiles commented Sep 27, 2013

I don't think this is the correct conclusion. The only time IDs are a particular problem is if you use DOM-level ID referencing, which is rare. <label for=...> is one of the very few times this comes up.

As long as you don't use <label for>, you shouldn't have problems. Yes, the generated DOM is technically invalid, but it's not otherwise detectable.

@sjmiles
Copy link
Contributor

sjmiles commented Sep 27, 2013

IMO, the FAQ should say something like:

Avoid using <label for> when using Polymer, as the ID may not resolve correctly when using the ShadowDOM polyfill.

@ebidel
Copy link
Contributor

ebidel commented Sep 27, 2013

Short and sweet. That's what I was looking for. Thanks
On Sep 26, 2013 8:22 PM, "Scott J. Miles" notifications@github.com wrote:

IMO, the FAQ should say something like:

Avoid using when using Polymer, as the ID may not resolve
correctly when using the ShadowDOM polyfill.


Reply to this email directly or view it on GitHubhttps://github.com//issues/295#issuecomment-25219971
.

@ebidel
Copy link
Contributor

ebidel commented Sep 27, 2013

http://www.polymer-project.org/faq.html#multipleids

On Fri, Sep 27, 2013 at 9:32 AM, Eric Bidelman ebidel@gmail.com wrote:

Short and sweet. That's what I was looking for. Thanks
On Sep 26, 2013 8:22 PM, "Scott J. Miles" notifications@github.com
wrote:

IMO, the FAQ should say something like:

Avoid using when using Polymer, as the ID may not resolve
correctly when using the ShadowDOM polyfill.


Reply to this email directly or view it on GitHubhttps://github.com//issues/295#issuecomment-25219971
.

@stefanjudis
Copy link
Author

👍

@sorvell sorvell closed this as completed Aug 11, 2014
@yurychika
Copy link

@ebidel the link http://www.polymer-project.org/faq.html#multipleids seems to be a 404.

@ebidel
Copy link
Contributor

ebidel commented Oct 19, 2015

@dkeeghan
Copy link

@ebidel A related conversation here involves the use of IDs in aria attributes like aria-labelledby and aria-describedby where you can have ids referencing other components. How would you suggest referencing these in this case (we can't just not use them)?

@mercmobily
Copy link

I landed here after snooping around wondering if using "id" in a component was a good idea. The FAQ is only there for 0.5 which is now rather obsolete.

This only creates a problem when doing DOM-level ID referencing. Before now, it was discussed that the only tag with the problem was <label for="..."> which I assume is obsoleted in favour of iron-label. But now... aria-labelledby seems to be yet another example of DOM-level ID referencing with a problem. Is it worthwhile creating another issue about these?

(I really hope to open this ticket in 5 years time and think "ahhhhhh do you remember the days when we had to use shims and polyfills"?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants