1683303 - Remove Thunderbird usage of XUL equalsize="always"

Status: RESOLVED FIXED

(Thunderbird :: General, task, P2)

Description

[Magnus Melin [:mkmelin]]

+++ This bug was initially created as a clone of Bug #1683296 +++

Remove Thunderbird usage of XUL equalsize="always" like bug 1683296 for Firefox.

https://searchfox.org/comm-central/search?q=equalsize&path=mail&case=false&regexp=false 23 hits
For AccountWizard.xhtml, we should get rid of that. It's only for movemail and nntp. Movemail support is going away, and we should be able to have something quite more simple for nntp.

https://searchfox.org/comm-central/search?q=equalsize&path=calendar&case=false&regexp=false 8 hits

Comment 1

[Magnus Melin [:mkmelin]]

For some of these cases, we may want to adjust the markup and not just the styling. As long as the end result looks ok. (It may not be very polished atm.)

Comment 2

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Calendar week view with equalsize="always" (current behaviour) when we have excess space

Comment 3

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Calendar week view with equalsize="always" (current behaviour) when we have less space

Comment 4

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Calendar week view with unset equalsize and no extra styling when we have less space

Comment 5

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Calendar week view with unset equalsize and flex box styling when we have excess space

Comment 6

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Calendar week view with unset equalsize and flex box styling when we have less space

Comment 7

[Henry Wilkes (they/them) [:henry-x]]

Just placing some notes here with some of my initial ideas for the more important cases, since I haven't been able to find a direct replacement in CSS styling for the equalsize="always" property.

I looked into this for the calendar week view. There, we have the XUL:

<!-- Days of the week labels -->
<box class="labelbox multiday-view-label-box">
    ...
    <box class="labeldaybox multiday-view-label-day-box"  flex="1" equalsize="always">
        <!-- Monday -->
        <calendar-day-label/>
        <!-- Tuesday -->
        <calendar-day-label/>
        ...
    </box>
    ...
</box>
<!-- All-day events -->
<box class="headerbox multiday-view-header-box">
    ...
    <box class="headerdaybox multiday-view-header-day-box" flex="1" equalsize="always">
        <!-- Monday -->
        <calendar-header-container/>
        <!-- Tuesday -->
        <calendar-header-container/>
        ...
    </box>
    ...
</box>
<!-- Hour-duration events -->
<scrollbox>
    ...
    <box class="daybox multiday-view-day-box" flex="1" equalsize="always">
        <!-- Monday -->
        <calendar-event-column/>
        <!-- Tuesday -->
        <calendar-event-column/>
        ...
    </box>
    ...
</scrollbox>

Sizing with equalsize=always

So it seems that a horizontal XUL box with equalsize="always" will request the widths:

box-width = max(max(min-widths-of-children) * num-children, parent-width)
child-width = box-width / num-children

So each child will be at least as wide to fit the widest content. Moreover, the XUL box will request more space in order to achieve this. Note that the parent-width is a rough approximation of what the width of the parent would be if it didn't contain the box.

The current week view relies on this to create a table-like effect. Each XUL box share the same parent, so when one of them requires more space, it will increase the parent-width for the other boxes, so they all end up having the same child widths. See https://bugzilla.mozilla.org/attachment.cgi?id=9208482 and https://bugzilla.mozilla.org/attachment.cgi?id=9208483

Sizing without equalsize=always

After removing the equalsize="always", the alignment messes up. See https://bugzilla.mozilla.org/attachment.cgi?id=9208484

Flex box styling

An obvious fix might have been to style the XUL box as a flex box:

.multiday-view-label-day-box, .multiday-view-header-day-box, .multiday-view-day-box {
  display: flex;
}

.multiday-view-label-day-box > *, .multiday-view-header-day-box > *, .multiday-view-day-box > * {
  flex: 1 1 0;
}

The flex property means that each item grows and shrinks at the same rate (1 1), and each one starts with an initial size of 0. This does mean that when we have enough space, each item will receive the same amount of space. See https://bugzilla.mozilla.org/attachment.cgi?id=9208485

However, when there is restricted space, the styling will not cause the flex box to request more space to maintain equal width children. Instead, the behaviour is roughly:

box-width = max(sum(min-widths-of-children), parent-width)
child-width = max(min-width, box-width / num-children)

What this means for the calendar week view is that some entries will be wider, and will become misaligned with the other XUL boxes, breaking the table effect. In particular, you can see this with the day labels, since the text width is variable between them. See https://bugzilla.mozilla.org/attachment.cgi?id=9208486

A "fix" for this is to set the css width to be 0 for each item, but all this means is that their content overflows. I can't find a CSS property to make the flex box request more space to keep the children equal sized. I'm less familiar with the grid display, but I couldn't get that to work either.

Potential fixes

Here are some fixes I'm aware of.

1. Use javascript

We could make the XUL box listen to each of its children's minimal widths through CSS style min-width, and then set its own min-width to be max(min-widths-of-children) * num-children.

2. Use markup

Turn the calendar week view into an actual table, with

.multiday-view-label-day-box, .multiday-view-header-day-box {
  position: sticky;
}

This would have the benefit of the markup being more literal to what is seen. However, this only technically fixes part of the problem: the columns will always be aligned, but they will not necessarily have the same width when the space is limited since the width of the table will be roughly:

table-width = max(sum(min-widths-of-columns), parent-width)
column-width = max(min-width, table-width / num-children)

However, unlike misaligned cells, having variable widths isn't an issue to reading the information you need. And we could always do the same thing as in fix 1. if this is important.

Other fixes?

Any other ideas on how you would want to proceed in this case.

Comment 8

[Henry Wilkes (they/them) [:henry-x]]

ok, I was able to get the equalsize effect using the grid layout.

Here's a demo:

<div class="scroll">
    <div class="row">
      <div>Monday</div>
      <div>Tuesday</div>
      <div>Wednesdaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaay</div>
    </div>
    <div class="row">
      <div>Mondaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaay</div>
      <div>Tuuuuuuuuuesday</div>
      <div>Wed</div>
    </div>
</div>

with

.row {
    display: grid;
    grid-auto-columns: 1fr;
    grid-auto-flow: column;
    min-width: max-content;
}

.row > div {
  border: black solid 1px;
  text-align: center;
  padding: 5px;
}

.scroll {
  overflow: scroll;
  display: grid;
}

This is the most minimal I could get it down to. In particular, I found that I had to give the scroll the grid layout in order to get both of its children to take up the full horizontal space within the scroll box. I'm not 100% sure why this works at the moment, will have to think about it.

@aleca Is this similar to what you did before?

Comment 9

[Alessandro Castellani [:aleca]]

Yes, this is similar to the solution I used.
In my case I needed a consistent 3 column structure so it's a bit different, but the concept is the same.
https://searchfox.org/comm-central/rev/5957a014611d5adf37557044215b486cce565856/mail/themes/shared/mail/accountManage.css#457-463

I found that I had to give the scroll the grid layout in order to get both of its children to take up the full horizontal space within the scroll box. I'm not 100% sure why this works at the moment, will have to think about it.

I had similar problems. I think is due to the fact that we inherit CSS from m-c which applies a display: -moz-box to the divs. At least that was the reason for the issues I had, but I'm not sure it applies here as well.

Since we're in full HTML here, it would be nice to use some semantic element as it makes the structure more readable when dealing with many children and repeated grids.
We could use <section> for the rows and <aside> for the columns.

Maybe this is a silly question but, is using a <table> not a viable solution for this?

Comment 10

[Magnus Melin [:mkmelin]]

Indeed the semantically correct element is probably a <table> for this case. I'm not sure how easy that is to change though. Keep in mind that the view can also be rotated. (View | Calendar | Current View | Rotate)

Comment 11

[Henry Wilkes (they/them) [:henry-x]]

I tried implementing the styling in the demo I shared into the weekview calendar, using the existing DOM, but it wasn't working. I think the issue is the structure isn't so simple as in the demo. It might require some wrapping elements.

(In reply to Alessandro Castellani [:aleca] from comment #9)

Since we're in full HTML here, it would be nice to use some semantic element as it makes the structure more readable when dealing with many children and repeated grids.
We could use <section> for the rows and <aside> for the columns.
Maybe this is a silly question but, is using a <table> not a viable solution for this?

The problem is that table doesn't have much customization for dynamic resizing. In particular, I couldn't get it have dynamic have equal width columns (the same issue as the flexbox).

(In reply to Magnus Melin [:mkmelin] from comment #10)

Indeed the semantically correct element is probably a <table> for this case. I'm not sure how easy that is to change though. Keep in mind that the view can also be rotated. (View | Calendar | Current View | Rotate)

That's good to know. Thanks! Another reason to avoid the table display.

To get the table structure, with the equalsize effect, and being rotate-able, we can just restyle as a grid. Demo:

<div class="scroll">
  <table id="table">
    <tbody>
      <tr class="row">
        <th class="day time"></th>
        <th class="day">Monday</th>
        <th class="day">Tuesdaaaay</th>
        <th class="day">Wed</th>
      </tr>
      <tr class="row">
        <th class="time">12:00</th>
        <td class="event">event 1</td>
        <td class="event"></td>
        <td class="event">event 2</td>
      </tr>
      <tr class="row">
        <th class="time">13:00</th>
        <td class="event"></td>
        <td class="event">event 3</td>
        <td class="event">event 4</td>
      </tr>
    </tbody>
  </table>
</div>
<button onclick="document.getElementById('table').toggleAttribute('rotate')">Rotate</button>

.scroll {
  overflow: scroll;
  resize: both;
}

table {
  display: grid;
  min-width: max-content;
  box-sizing: content-box;
}

table:not([rotate]) {
  grid-auto-flow: row;
  grid-template-columns: auto repeat(3, 1fr);
}

table[rotate] {
  grid-auto-flow: column;
  grid-template-rows: auto repeat(3, 1fr);
}

tbody {
  display: contents;
}

tr {
  display: contents;
}

table:not([rotate]) .day {
  position: sticky;
  top: 0;
}

table[rotate] .time {
  position: sticky;
  top: 0;
}

th {
  background: lightgrey;
}

th, td {
  text-align: center;
  border: black solid 1px;
  padding: 2px;
}

Unfortunately, the restyling (display: grid) destroys the accessibility tree: https://developer.mozilla.org/en-US/docs/Web/CSS/display#tables

But I think it can be put back in by respecifying the accessibility roles, if that's something thunderbird wants to eventually support.

Comment 12

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1683303 - Remove XUL equalsize from email Wizard. r=aleca

Comment 13

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1683303 - Remove XUL equalsize from address book. r=mkmelin

There does not seem to be a good reason to require the phone numbers to take up as much space as the email and name details.

Comment 14

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1683303 - Remove XUL equalsize from composition editor. r=aleca

Comment 15

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1683303 - Remove XUL equalsize from instant messenger. r=aleca

Depends on D111078

Comment 16

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1683303 - Remove XUL equalsize from old account wizard. r=aleca

Depends on D111079

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/841a2205d072
Remove XUL equalsize from email Wizard. r=aleca
https://hg.mozilla.org/comm-central/rev/9b2de929a378
Remove XUL equalsize from address book. r=mkmelin

Comment 18

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1683303 - Remove XUL equalsize from general preferences. r=aleca

The content-type/action "table" was replaced with an actual HTML table, which is more accessible.

Removed the previous behaviour where the table "headers" became misaligned with the "cells" when scrolling was needed. Now, fixed with the table layout and sticky headers.

Removed the previous behaviour where two clicks/tab-presses were required to change the action: one to select the row and reveal the actual menulist, and another click to open the menulist. Now, the menulist is always visible and focusable.

Replaced header clicks for sorting, which was not focusable or interpretable for screen readers, with a separate select element.

Comment 19

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1683303 - Remove XUL equalsize from event attendee box. r=mkmelin

Using a grid with a flexible number of columns replaces the previous behaviour.

Comment 20

[Henry Wilkes (they/them) [:henry-x]]

(In reply to Henry Wilkes [:henry] from comment #19)

Created attachment 9214674 [details]
WIP: Bug 1683303 - Remove XUL equalsize from event attendee box.

Using a grid with a flexible number of columns replaces the previous behaviour.

Still having problems with this one. Specifically, the previous behaviour was using javascript to listen for a window resize and rebuilding the attendee box from scratch if the number of attendees that could fit in a row changed! I was able to get rid of this code and just change the styling for item-attendees-box to be a dynamic grid:

.item-attendees-box {
  display: grid;
  grid-template-columns: repeat(auto-fit, minmax(200px, 1fr));
  min-height: 54px; /* Height of two rows */
}

However, whilst this works well in HTML, when placed with XUL elements, the grid was being assigned too much space: roughly double the height it actually uses.

@mkmelin suspected that a parent with display: -moz-box was causing the problem. I think this is indeed the source of the problem. However, I found that for the event summary dialog I had to change the display of all ancestor nodes up to the <dialog> element.

Even then, for the event summary dialog there is still a sizing issue. For example, consider that we have 8 attendees and two cases for the dialog:

1. Dialog at minimal width, which fits 2 attendees
2. Dialog stretched by user to fit 3 attendees

In case 1, we would expect the list of attendees to occupy 2 grid columns and 4 rows. In case 2, we would expect the list of attendees to occupy 3 grid columns and 3 rows. With my changes, this does happen. However, in case 2, despite the grid requiring less rows, the ancestor XUL <calendar-item-summary> still assigns the same height as in case 1, and cannot be shrunk through a user dialog resize. So there is a gap between the list of attendees and the "Edit" button. Note that the HTML element with the grid styling has the correct height in both cases, it is just the ancestor that is not properly taking the new height into account.

If this can't be fixed, some solutions would be:

1. Overwrite CSS display for all the ancestors. This would be easier for the event summary dialog, but not for event editing where the attendee box is also used. Plus, this only solves half the problem.
2. Keep the javascript code that listens for a resize, but instead of rebuilding we just change the element's style.gridTemplateColumns to repeat(${Math.floor(containerWidth / 200)}, 1fr). I don't like this because I think sizing should generally be left up to CSS, and we already have a CSS styling that should do this.
3. Just make the grid fixed at two columns. This is the default anyway and I'm not sure if anyone relies on three or more columns.
4. Just use a single column. I would actually prefer a single column over two columns. Whilst the display is more compact with two or more columns, I think it is much easier to read and scan the list of attendees as a single column. And for long attendee lists (more than 10?), we could provide sorting and filtering. Also, in the back of my mind is the attendee icon's current lack of accessibility (Bug 1702560). I think a single column would offer better solutions for that (e.g. if the attendees have a dropdown box which opens on focusing, which contains the information that is currently in the tooltip text).

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9565c3d62855
Remove XUL equalsize from composition editor. r=aleca
https://hg.mozilla.org/comm-central/rev/89dc7f66cf60
Remove XUL equalsize from instant messenger. r=aleca
https://hg.mozilla.org/comm-central/rev/a1b6f5beae7f
Remove XUL equalsize from old account wizard. r=aleca
https://hg.mozilla.org/comm-central/rev/a0b86bbaaa09
Remove XUL equalsize from general preferences. r=aleca

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/73bbd0e671a1
Remove XUL equalsize from event attendee box. r=darktrojan,mkmelin

Comment 23

[Rob Lemley [:rjl]]

Thunderbird 91.0 (backout):
https://hg.mozilla.org/releases/comm-beta/rev/ba37ec014315

Comment 24

[Henry Wilkes (they/them) [:henry-x]]

Last equalsize removed in https://hg.mozilla.org/comm-central/rev/ea72f328b8be (bug 1713130).

Comment 25

[Magnus Melin [:mkmelin]]

Yay! \o/