Show "Concealed fact" tag on transparent facts

[paulwarren-wk]

Reason for change

Some preparers make the contents of iXBRL tags invisible, so that the visual appearance of the fact can be provided by different HTML elements. The viewer currently detects where this is done using display: none and puts a "concealed fact" tag in the inspector. We've recently discovered preparers doing the same thing, but using a transparent font color for the tag.

This is poor tagging practice, as it removes the connection between the tagged and displayed values, and interferes with the behaviour of viewers.

Description of change

This PR extends the check for concealed facts to include facts with a transparent font (which we define as an alpha value of less than 10%).

Steps to Test

Create a viewer for this filing

Do a search for "Proft loss before tax". Scroll through the results and you should see facts highlighted as "Concealed facts"

Double click on one such result.

The fact inspector should also have a "Concealed fact" tag.

Now unselect this fact (e.g. click on the background). Note that you can't click on the number to re-select the fact. This is because the tagged fact is underneath another HTML element which is what is visible.

review:
@Arelle/arelle
@paulwarren-wk

[aviary3-wk]

Security Insights

No security relevant content was detected by automated scans.

Action Items

• Review PR for security impact; comment "security review required" if needed or unsure
• Verify aviary.yaml coverage of security relevant code

---

Questions or Comments? Reach out on Slack: #support-infosec.

[brettkail-wk]

I guess the isNaN check is redundant, but it's perhaps clearer to be explicit?

[paulwarren-wk]

I hadn't spotted that it's redundant, but I do think it is worth being explicit, as it documents that NaN is expected on some inputs, and makes it clear how it's handled.

[brettkail-wk]

This is fine, but I think you could use .some instead?

[paulwarren-wk]

It's a jQuery object, so it'd need a toArray(), but I had forgotten that jQuery's .is() can take a function (2a46728).

[brettkail-wk]

I always forget about the jQuery wrappers... Out of curiosity, do you think it's still worth using?

[paulwarren-wk]

A good question. We do have an issue to remove jQuery usage (#378) but we've subsequently decided that it's not a priority. I think some of the big reasons for using jQuery have gone (e.g. abstracting browser incompatibilities, more convenient iteration constructs).

I have converted some code to be pure JS, in particular parts of preProcessiXBRL which is a performance-critical part of document loading, although I'm not sure it made much of a difference. Interestingly, I tried to replace the really critical $(n).wrap(wrapper); line with pure JS, and could only make things slower.

There's some stuff which still seems somewhat cumbersome in pure JS. For example:

$(n).css('display')

vs

getComputedStyle(n).getPropertyValue('display')

I also find DOM creation quite cumbersome:

const div = document.createElement('div');
div.append(document.createTextNode(this.fact.readableValue()));

vs

const div = $("<div></div>").text(this.fact.readableValue());

That said, I don't consider myself a JS expert, and always feel like I'm some years behind the curve, so there may well be better ways to do this. Perhaps there are better, lighter-weight helper libraries that we could be using instead?

[brettkail-wk]

A good question. We do have an issue to remove jQuery usage (#378) but we've subsequently decided that it's not a priority.

Thanks, I thought I remembered something. Perhaps not a priority, though I'll say my knowledge of JS is weak and jQuery even weaker, so I find myself switching between MDN and jQuery docs while reviewing these PRs and struggling to understand what's happening with an extra library layer around the DOM...

Interestingly, I tried to replace the really critical $(n).wrap(wrapper); line with pure JS, and could only make things slower.

Interesting indeed. There must be something jQuery was doing (different DOM APIs, caching, ...) to make up for the extra abstraction layer since "jquery vs dom performance" search results generally look unfavorable. That we can't explain what the extra layer is doing is also unsettling...

There's some stuff which still seems somewhat cumbersome in pure JS. For example:

Presumably we could write small getCss(e) and appendText(e, text) utils to help the particularly verbose replacements.

[paulwarren-wk]

Thanks, I thought I remembered something. Perhaps not a priority, though I'll say my knowledge of JS is weak and jQuery even weaker, so I find myself switching between MDN and jQuery docs while reviewing these PRs and struggling to understand what's happening with an extra library layer around the DOM...

True, and jQuery is a real pain for interactive debugging.

Interestingly, I tried to replace the really critical $(n).wrap(wrapper); line with pure JS, and could only make things slower.

Interesting indeed. There must be something jQuery was doing (different DOM APIs, caching, ...) to make up for the extra abstraction layer since "jquery vs dom performance" search results generally look unfavorable. That we can't explain what the extra layer is doing is also unsettling...

Yes. There's not a trivial pure-JS equivalent for that one, and there seem to be a number of different approaches to replicating it. We use it a lot on massive documents, so performance is critical. For its faults, jQuery is mature and presumably someone has spent time figuring out how to make this fast. I'm sure we could figure out how jQuery does it from the source and replicate it.

The jQuery wrappers must have an overhead but from the limited work I've done on it, it seems to have a relatively small impact on the operations that we do in the viewer.

Presumably we could write small getCss(e) and appendText(e, text) utils to help the particularly verbose replacements.

Yes, although I feel that someone else must have already done this. I should probably spend some more time looking for alternatives.