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

Disable check hyphens flag #90

Closed
wants to merge 2 commits into from
Closed

Conversation

Sebmaster
Copy link
Member

No description provided.

@Sebmaster Sebmaster requested a review from domenic May 27, 2017 13:02
@domenic
Copy link
Member

domenic commented May 31, 2017

I've updated this to run the new tests in web-platform-tests/wpt#5976 and am getting four failures where it should be throwing an exception but doesn't:

      1) ‍.example (CheckJoiners is true)
      2) xn--1ug.example
      3) يa (CheckBidi is true)
      4) xn--a-yoc

Any ideas, @Sebmaster @rmisev?

@Sebmaster
Copy link
Member Author

Checkjoiners is probably a bug, checkbidi has never been implemented. Gotta dig into why the other two should fail/do not.

@rmisev
Copy link
Contributor

rmisev commented May 31, 2017

(1) actually is "\u200D.example"
(2) is the "\u200D.example" in Punycode
(4) is the "يa" in Punycode
So (2) and (4) must return an error the same as (1) and (3).
To pass tests the CheckJoiners and CheckBidi must be set to true (unfortunately, not implemented in the Sebmaster/tr46.js).

@domenic
Copy link
Member

domenic commented May 31, 2017

Hmm OK. I wonder if we can get that implemented soon, or if we should just sign-off on the spec changes as we've tested them as best we can.

@Sebmaster
Copy link
Member Author

Ah, CheckJoiners enables ContextJ rules, that's also never been implemented.

@Sebmaster Sebmaster closed this Jul 27, 2017
@domenic domenic deleted the feature/disable-check-hyphens branch December 5, 2017 22:39
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

Successfully merging this pull request may close these issues.

3 participants