Fix conditional reveal fullstop.

[nataliecarey]

Fixes #1808

While working with an HMRC service team I discovered this bug. The service had used IDs with a full stop in them implying hierarchy. The format was roughly question1.option1. This is valid HTML but due to the selectors in the JS this was turning into the selector #questoin1.option1 which looks for an element with an ID of question1 as well as a class of option1. This change escapes the full stop to make the query #question1\.option1 which will look for an ID of question1.answer1.

In the process of fixing this it became clear that some of the examples and tests (as well as real production code) have situations where a selector of #null will be used, this occurs when some items have conditional reveal and others don't. I've checked for this case and therefore #null will no longer be used.

This is intended to be backward compatible and to only fix the bugs related to the issues we've experienced in one of our services.

[36degrees]

Hey @natcarey,

Thanks for raising this. Based on your explanation and the code change I think that this is the same issue as #1808. Can you take a look and see if you agree?

We've got an alternate fix which we've previously explored – switching to using document.getElementById means we can avoid needing to escape characters in the ID which could be interpreted as CSS selector syntax. This should address the issue you're seeing with full stops in IDs, but also works for IDs that include other characters like [ and ].

Unfortunately, because this changes the scope of the lookup from the $module to the document, we're treating that fix as a breaking change as there's a chance that users may be using non-unique IDs and – even though that's technically invalid – this would break conditional reveals for those users.

However, we were already planning to include that fix in 4.0 which is now likely to be the next release – at the minute we have no plans to do any more 3.x releases.

How does that sound? Can you wait for a proper fix in the 4.0 release?

[nataliecarey]

There are a few answers here:

1. I don't think this is urgent, the team are looking to replace full stops with hyphens - I'm not sure how that's going but it's likely to be fine. If you're not doing any more 3.x releases I'm imagining there'll be a 4.0 release soon - if that's the case then I'm pretty sure we can wait for it.
2. This shouldn't change the scope - it only change the construction fo the selector, I think that's a more appropriate fix than getElementByID (which I used to prove the concept and then moved away from)
3. This also fixes the issue where #null is used as a selector - I don't think that's likely to cause a problem in

I'm going to update the PR to include [ and ] as well confirming that other valid special characters - that way you've got an option which supports those two cases without a breaking change so it can be used if you want to include it before 4.0.

[nataliecarey]

I've updated this to deal with all special chars, to be clear this is a nonbreaking change and doesn't change the scope of the selector.

[vanitabarrett]

Not de-scoping this from v4.0.0, but removing it from the milestone as the associated issue is already linked there.

[36degrees]

We're going to go with the approach in #2370 instead – replacing the calls to $module.querySelector with document.getElementById.

Thanks again @natcarey – really appreciate you taking the time to put this together.