v1.14.1: regression in tabs with non-standard IDs

[tagliala]

Hello,

not sure that this is an issue with jQuery UI, but I think it is worth to report it

In Active Admin, we were trying to upgrade jQuery UI to 1.14.1 from 1.13.2, but one of our specs related to the tabs component was failing.

The failure is related to the way Active Admin creates the fragment name when it contains just an emoji, which is the same way GitHub creates fragments in markdown to HTML conversion

Example: ☺️ => %EF%B8%8F

Reproducible test case:

Code taken from: https://jqueryui.com/tabs/

• ✅ v1.14.0 https://jsfiddle.net/tagliala/dkLa4v7e/8
• ❌ v1.14.1 https://jsfiddle.net/tagliala/dkLa4v7e/9

Expected

Actual

Refs:

• Upgrade vendored jQuery UI to 1.14.1 [Pending upstream release] activeadmin/activeadmin#8670
• https://github.com/tagliala/rubyzip/wiki#%EF%B8%8F

[mgol]

Thanks for the report and easy to run test cases!

This was caused by #2307. I'll have a look at it for 1.14.2.

[mgol]

I added decoding in #2307 as it's needed for some complex characters like emojis to work - it's interesting that the linked issue claims it's emoji handling that's now broken... 🤔

[mgol]

I wanted to see if we should or should not be decoding the anchor hash value in the end. I checked native browser behavior, without jQuery UI - one was testing whether <a href="#🤗"> will scroll to <div id="🤗">, the second one was whether <a href="#😅"> will scroll to <div id="%F0%9F%98%85">. Since anchor.hash is always encoded:

anchor1.hash // '#%F0%9F%A4%97' for `<a href="#🤗">`
anchor2.hash // '#%F0%9F%98%85' for `<a href="#😅">`

we have the following:

1. If and only if we do decode, href from <a href="#🤗"> will point to the id of <div id="🤗">
2. If and only if we do NOT decode, href from <div id="🤗"> will point to the id of <div id="%F0%9F%98%85">

However, in the browser, clicking any of them will scroll to the proper div. 🤔

That got me curious so I looked into the specs. It starts with the "scrolling to a fragment" section of the HTML spec. That uses a concept of document's indicated part. Slightly below there's an algorithm to compute the indicated part. The interesting parts are steps 4 to 9:

4. Let potentialIndicatedElement be the result of finding a potential indicated element given document and fragment.
5. If potentialIndicatedElement is not null, then return potentialIndicatedElement.
6. Let fragmentBytes be the result of percent-decoding fragment.
7. Let decodedFragment be the result of running UTF-8 decode without BOM on fragmentBytes.
8. Set potentialIndicatedElement to the result of finding a potential indicated element given document and decodedFragment.
9. If potentialIndicatedElement is not null, then return potentialIndicatedElement.

First, in steps 4-5, the algorithm tries the hash as-is, without decoding. Then, if one is not found, the same is attempted with a decoded hash. Yes, it actually checks both!

That complicates things as we cannot compute the id from the hash without running a query and checking if an element with a matching id is actually in the DOM. 🤕 As part of that, we should also perhaps revert 1e5662e (this part should also be tested).

[mgol]

@tagliala I have a fix up at #2345, could you verify it fixes the issues you have?