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

Avoid interfering with layout of tables with absolutely positioned content #641

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

paulwarren-wk
Copy link
Contributor

Fixes #632

Reason for change

The viewer currently interferes with the layout of tables that contain absolutely positioned content. The problematic situation is this:

    <div class="page" style="position: absolute">
        <table>
            <tr>
                <td>
                    <div style="position: absolute">1,000</div>
                </td>
            </tr>
        </table>
    </div>

The viewer then adds position: relative to the <table> tag, which means that the absolutely position <div> is now positioned with respect to the wrong parent.

The viewer adds this class, because it is used to place the table export icon in the top left of the table.

Description of change

The viewer no longer adds an export handle if absolutely positioned content is found within the table.

It would obviously better to enable the export functionality, as the presence of the table tags means that the export should work (where previously it didn't on most ESEF filings), but it's not at all trivial, as the <table> tag is likely to be rendered is some random location.

Steps to Test

Check that the layout of the sample document on #632 is now correct when loaded in the viewer.

review:
@Arelle/arelle
@paulwarren-wk

@paulwarren-wk paulwarren-wk requested a review from a team as a code owner March 10, 2024 17:28
@aviary-wf
Copy link

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.

//
// Where a table has absolutely positioned content, it's quite
// likely that the table tag itself is positioned in a random
// place.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worry this will break/remove the handle from Workiva-generated documents, which use position:absolute on all <div> within a <table> but I believe position the <table> in a specific/non-random place (test).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it looks like the table has been carefully constructed to ensure that it's position and size matches the rendered content, so in this case if we were to put the handle at the top left of the table, it would work.

We could potentially detect that the table has non-zero size, and show the handle in that case, but we still have the issue that adding position: relative may interfere with the layout. In this example, the cell contents are positioned relative to an enclosing <div> which happens to have the same position as the <table> element, so adding position: relative has no impact. But, an innocuous change like adding a top margin to the table, means that adding position: relative breaks the layout because the cell contents are positioned relative to a different location.

We could find the minimum top and left positions of all the absolutely positioned children, and put the export handle at that location, although we'd need to take care that the children are actually position relative to the table or some parent.

Elsewhere, we've discussed replacing all highlighting / borders / annotation with constructs that are positioned relative to to the top of the document, but would be a significant change, and would require recalculating positions if the window gets resized.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw a link to this blog post over the weekend, which seemed promising:

Once you can use this freely, you’ll have to care less about exact DOM positioning of elements (aside from accessibility concerns). The way it is now, the element you want to position relative to another has to be a child element and for there to be a positioning context to work within. This can dictate where elements go in the DOM, whether or not that makes sense.

Looks like Firefox intends to prototype, which is promising.

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.

[BUG] Table Style : Position relative is added automatically
4 participants