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

Unexpected behavior with table and container navigation for nested tables #7382

Closed
leonardder opened this Issue Jul 11, 2017 · 10 comments

Comments

Projects
None yet
5 participants
@leonardder
Collaborator

leonardder commented Jul 11, 2017

I've seen the case below in a corporate environment. I agree it isn't common style of coding.

Steps to reproduce:

  1. Execute the following from a Firefox location bar:

    data:text/html, <p>Prepare for a table</p><table><tr><th>Header 1</th><th>Header 2</th></tr><tr><td>Outer table cell 1</td><td>Outer table cell 2</td></tr><tr><td><table><tr><td>Inner table cell 1</td><td>Inner table cell 2</td></tr></table></td><td>Outer table cell 4</td></tr><tr><td>Outer table cell 5</td><td>Outer table cell 6</td></tr></table>
    
  2. Use table navigation inside the table. Everything works fine, until you reach outer table cell 3, which consists of a table itself with two cells.

Expected behavior:

A. When in the inner table, pressing comma moves you out of it to outer table cell 4
B. Table navigation doesn't get trapped into the inner table as soon as you are in it.

Actual behavior:

A. When in the inner table, pressing comma moves you out of it to outer table cell 6, so the bottom of the outer table
B. Table navigation gets trapped into the inner table as soon as you are in it.

Technical details

The outer table is treated as a content table, the inner table is treated as a layout table. This is why pressing comma in the inner table moves you to the edge of the outer table, since layout tables aren't treated as containers. If you enable "Include layout tables" in browse mode settings, issue A goes away.

Points to discuss:

At least there is an inconsistency here, as table nav works for layout tables, but they aren't treated as containers.

  • Should we disable table navigation altogether for layout tables inside content tables? I think we should consider this. May be we should even disable table navigation in all layout tables
  • If the former is too much, how about treating layout tables as containers, so if you get trapped with table nav, you can navigate out of the container to the outer table?
@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Jul 11, 2017

Contributor

I've always believed table navigation shouldn't consider layout tables when inclusion of layout tables is disabled. However, I believe @michaelDCurran made a case against this, hence the current behaviour. Mick, is this (still?) correct, and if so, can you explain why? @Qchristensen, thoughts?

Contributor

jcsteh commented Jul 11, 2017

I've always believed table navigation shouldn't consider layout tables when inclusion of layout tables is disabled. However, I believe @michaelDCurran made a case against this, hence the current behaviour. Mick, is this (still?) correct, and if so, can you explain why? @Qchristensen, thoughts?

@michaelDCurran

This comment has been minimized.

Show comment
Hide comment
@michaelDCurran

michaelDCurran Jul 12, 2017

Contributor

I may have thought this at one time, but today I would be fine with ignoring layout tables. I think from memory the issue was more around the complexity to do this, especially with Gecko as we use a hybrid of virtualBuffer and out-of-proc IA2. having said this, some of that code was slightly refactored when abstracting table support for browseMode, so it is worth looking into again. I certainly agree the difference between table navigation and container jumping is bad.

Contributor

michaelDCurran commented Jul 12, 2017

I may have thought this at one time, but today I would be fine with ignoring layout tables. I think from memory the issue was more around the complexity to do this, especially with Gecko as we use a hybrid of virtualBuffer and out-of-proc IA2. having said this, some of that code was slightly refactored when abstracting table support for browseMode, so it is worth looking into again. I certainly agree the difference between table navigation and container jumping is bad.

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Jul 12, 2017

Contributor

P2 because this can be quite problematic and layout tables are still (unfortunately) rather common.

Contributor

jcsteh commented Jul 12, 2017

P2 because this can be quite problematic and layout tables are still (unfortunately) rather common.

@jcsteh jcsteh added the p2 label Jul 12, 2017

@leonardder

This comment has been minimized.

Show comment
Hide comment
@leonardder

leonardder Jul 12, 2017

Collaborator

Ok, let me investigate the complexity of this later this week

Collaborator

leonardder commented Jul 12, 2017

Ok, let me investigate the complexity of this later this week

@michaelDCurran

This comment has been minimized.

Show comment
Hide comment
@michaelDCurran

michaelDCurran Jul 12, 2017

Contributor

I'll take this, as I am also working on some ARIA grid stuff which may directly affect this code.

Contributor

michaelDCurran commented Jul 12, 2017

I'll take this, as I am also working on some ARIA grid stuff which may directly affect this code.

@leonardder

This comment has been minimized.

Show comment
Hide comment
@leonardder

leonardder Jul 12, 2017

Collaborator

Ah, that works too.

Collaborator

leonardder commented Jul 12, 2017

Ah, that works too.

@leonardder

This comment has been minimized.

Show comment
Hide comment
@leonardder

leonardder Jul 19, 2017

Collaborator

Confirmed that this now works as expected.

Collaborator

leonardder commented Jul 19, 2017

Confirmed that this now works as expected.

@nvaccessAuto nvaccessAuto added this to the 2017.3 milestone Aug 2, 2017

@derekriemer

This comment has been minimized.

Show comment
Hide comment
@derekriemer

derekriemer Aug 2, 2017

Collaborator

take this example.

data:text/html, <p>Prepare for a table</p><table><tr><th>Header 1</th><th>Header 2</th></tr><tr><td>Outer table cell 1</td><td>Outer table cell 2</td></tr><tr><td><table><tr><th>Inner table cell 1</th><td>Inner table cell 2</td></tr></table></td><td>Outer table cell 4</td></tr><tr><td>Outer table cell 5</td><td>Outer table cell 6</td></tr></table>

Table nav still traps the user, which is annoying. Could we do something like "Leaving nested table" when navigating out of the inner table with table nav?

Collaborator

derekriemer commented Aug 2, 2017

take this example.

data:text/html, <p>Prepare for a table</p><table><tr><th>Header 1</th><th>Header 2</th></tr><tr><td>Outer table cell 1</td><td>Outer table cell 2</td></tr><tr><td><table><tr><th>Inner table cell 1</th><td>Inner table cell 2</td></tr></table></td><td>Outer table cell 4</td></tr><tr><td>Outer table cell 5</td><td>Outer table cell 6</td></tr></table>

Table nav still traps the user, which is annoying. Could we do something like "Leaving nested table" when navigating out of the inner table with table nav?

@leonardder

This comment has been minimized.

Show comment
Hide comment
@leonardder

leonardder Aug 2, 2017

Collaborator

@derekriemer commented on 2 aug. 2017 17:14 CEST:

Table nav still traps the user, which is annoying. Could we do something like "Leaving nested table" when navigating out of the inner table with table nav?

You can leave the inner table with comma, so I think that is at least something. But I think that data tables inside data tables should be treated as layout tables.

Collaborator

leonardder commented Aug 2, 2017

@derekriemer commented on 2 aug. 2017 17:14 CEST:

Table nav still traps the user, which is annoying. Could we do something like "Leaving nested table" when navigating out of the inner table with table nav?

You can leave the inner table with comma, so I think that is at least something. But I think that data tables inside data tables should be treated as layout tables.

@derekriemer

This comment has been minimized.

Show comment
Hide comment
@derekriemer

derekriemer Aug 4, 2017

Collaborator

There could be a usecase, but it would be verry weird.

Collaborator

derekriemer commented Aug 4, 2017

There could be a usecase, but it would be verry weird.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment