aboutsummaryrefslogtreecommitdiff
path: root/89/index.md
blob: 6565f7d6d96683953c3c742584ad9f2e9cef1400 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
Title: Missing column in table with max-width and min-width
Author: rodarima
Created: Sat, 02 Mar 2024 20:59:11 +0000
State: closed

There should be three columns:
![kk](https://github.com/dillo-browser/dillo/assets/3866127/4ecc80a6-611c-44e6-be6a-664a5dd31724)

Reproducer:

```html
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
  <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
  <title>Test table in container with max-width and min-width</title>
    <style type="text/css">
      .main {
        background: lightgreen;
        max-width: 500px;
        min-width: 200px;
        padding: 10px;
      }
      table, th, td {
        padding: 0.25em;
        background: lightblue;
        border: solid 1px black;
      }
  </style>
</head>
<body>
  <div class="main">
    <table width="100%">
      <tr>
        <th>AAAAAA</th>
        <th>BBBBBB</th>
        <th>CCCCCC</th>
      </tr>
      <tr>
        <td>aaaaaa</td>
        <td>bbbbbb</td>
        <td>cccccc</td>
      </tr>
    </table>
  </div>
</body>
</html>
```

--%--
From: rodarima
Date: Sun, 10 Mar 2024 12:00:23 +0000

So, this problem is happening because the table gets an allocation smaller than the container div, even though the CSS style has a 100% width (the table `width` attribute is converted to a CSS value):

![image](https://github.com/dillo-browser/dillo/assets/3866127/db4cc206-3bf1-451a-b050-000a3810e2c6)

![image](https://github.com/dillo-browser/dillo/assets/3866127/07b5533d-4f7c-43e6-b18f-d4ea2d5feac1)


As the table doesn't have the `auto` value, it enters [this branch](https://github.com/dillo-browser/dillo/blob/d61bf779f41617bbc31c3c5697e9275a6fbb1bcd/dw/table.cc#L942-L952) when updating the table size, setting `totalWidthSpecified` to `true`:

```c++
   // CSS 'width' defined and effective?
   bool totalWidthSpecified = false;
   if (getStyle()->width != core::style::LENGTH_AUTO) {
      // Even if 'width' is defined, it may not have a defined value. We try
      // this trick (should perhaps be replaced by a cleaner solution):
      core::Requisition testReq = { -1, -1, -1 };
      correctRequisition (&testReq, core::splitHeightPreserveDescent, true,
                          false);
      if (testReq.width != -1)
         totalWidthSpecified = true;
   }
```

This flag causes it to enter [the "case 2" branch](https://github.com/dillo-browser/dillo/blob/d61bf779f41617bbc31c3c5697e9275a6fbb1bcd/dw/table.cc#L1049) of the size calculation for tables:

```c++
   } else if (totalWidthSpecified && totalWidth > maxWidth) {
      DBG_OBJ_MSG ("resize", 1,
                   "case 2: totalWidthSpecified && totalWidth > maxWidth");

      // The width is specified (and so enforced), but all maxima sum
      // up to less than this specified width. The columns will have
      // there maximal width, and the extra space is apportioned
      // according to the column widths, and so to the column
      // maxima. This is done by simply passing MAX twice to the
      // apportioning function.

      // When column widths are specified (numColWidthSpecified > 0,
      // as calculated in forceCalcColumnExtremes()), they are treated
      // specially and excluded from the apportioning, so that the
      // specified column widths are enforced. An exception is when
      // all columns are specified: in this case they must be
      // enlargened to fill the whole table width.
```

--%--
From: rodarima
Date: Sun, 10 Mar 2024 12:08:27 +0000

The table doesn't have any width specification among columns (`numColWidthSpecified` is 0), so it enters this [branch](https://github.com/dillo-browser/dillo/blob/d61bf779f41617bbc31c3c5697e9275a6fbb1bcd/dw/table.cc#L1067-L1073), which calls the `Table::apportion2()` method:

```c++
      if (numColWidthSpecified == 0 ||
          numColWidthSpecified == colExtremes->size()) {
         DBG_OBJ_MSG ("resize", 1,
                      "subcase 2a: no or all columns with specified width");
         apportion2 (totalWidth, 0, colExtremes->size() - 1, MAX, MAX, NULL,
                     colWidths, 0);
      } else {
```
With this arguments (this is the first call):
```
dw::Table::apportion2 (this=0x5555558452a0, totalWidth=488, firstCol=0, 
  lastCol=-1, minExtrMod=dw::Table::MAX, 
  maxExtrMod=dw::Table::MAX, extrData=0x0, 
  dest=0x5555559185c0, destOffset=0)
```
The `totalWidth` is still correct, as it comes from the 500 px of the parent div, minus some border width. As `lastCol=-1` and `firstCol=0` no cell width computation is done (the table has no cells yet). It returns to `dw::Table::forceCalcCellSizes()` without modifying any cell sizes.

--%--
From: rodarima
Date: Sun, 10 Mar 2024 12:41:48 +0000

After `dw::Table::forceCalcCellSizes()` returns to `dw::Table::sizeRequestSimpl`, a [requisition correction is made](https://github.com/dillo-browser/dillo/blob/d61bf779f41617bbc31c3c5697e9275a6fbb1bcd/dw/table.cc#L112-L125):

```c++
   /**
    * \bug Baselines are not regarded here.
    */
   requisition->width =
      boxDiffWidth () + (numCols + 1) * getStyle()->hBorderSpacing;
   for (int col = 0; col < numCols; col++)
      requisition->width += colWidths->get (col);

   requisition->ascent =
      boxDiffHeight () + cumHeight->get (numRows) + getStyle()->vBorderSpacing;
   requisition->descent = 0;

   correctRequisition (requisition, core::splitHeightPreserveDescent, true,
                       false);
```

With the following values (before the `correctRequision()` call:

```
(gdb) p *requisition
$37 = {width = 12, ascent = 12, descent = 0}
```

This is taken by `dw::core::Widget::correctRequisition()` with `parent != NULL` and `quasiParent == NULL`, so [it delegates it to the parent](https://github.com/dillo-browser/dillo/blob/d61bf779f41617bbc31c3c5697e9275a6fbb1bcd/dw/widget.cc#L802-L808):
```c++
   } else if (parent) {
      DBG_OBJ_MSG ("resize", 1, "delegated to parent");
      DBG_OBJ_MSG_START ();
      parent->correctRequisitionOfChild (this, requisition, splitHeightFun,
                                         allowDecreaseWidth,
                                         allowDecreaseHeight);
      DBG_OBJ_MSG_END ();
```

The parent (the `div` which is a `dw::Textblock`) then calls `dw::core::Widget::correctReqWidthOfChild()` and then `dw::core::Widget::calcFinalWidth`:

```
(gdb)
dw::core::Widget::calcFinalWidth (this=0x5555558449f0, style=0x555555912f40,
  refWidth=-1, refWidget=0x555555849d70,
  limitMinWidth=10, forceValue=false, finalWidth=0x7fffffffd3bc)
    at ../../dw/widget.cc:938
938	   int width = calcWidth (style->width, refWidth, refWidget, limitMinWidth,
(gdb) p *finalWidth
$56 = 12
```

--%--
From: rodarima
Date: Sun, 10 Mar 2024 12:53:29 +0000

In `dw::core::Widget::calcFinalWidth()`, the width is computed by the CSS style which indicates `width: 100%`:
```
(gdb) p (100. * dw::core::style::perLengthVal_useThisOnlyForDebugging(cssValue))
$68 = 100
```
In `dw::core::Widget::calcWidth()`, `refWidth == -1` so the table [asks the div to provide the available width](https://github.com/dillo-browser/dillo/blob/d61bf779f41617bbc31c3c5697e9275a6fbb1bcd/dw/widget.cc#L906-L914):

```c++
      else {
         int availWidth = refWidget->getAvailWidth (forceValue);
         if (availWidth != -1) {
            int containerWidth = availWidth - refWidget->boxDiffWidth ();
            width = misc::max (applyPerWidth (containerWidth, cssValue),
                               limitMinWidth);
         } else
            width = -1;
      }
```

The value `width` is set to `220` by [calcFinalWidth()](https://github.com/dillo-browser/dillo/blob/d61bf779f41617bbc31c3c5697e9275a6fbb1bcd/dw/widget.cc#L680-L683) when constraining by the div min-width value:

```c++
   /* Constraint the width with min-width and max-width */
   calcFinalWidth (getStyle (), refWidth, NULL, 0, forceValue, &width);
   if (width == -1)
      width = refWidth;
```
So here is where it goes wrong.

--%--
From: rodarima
Date: Sun, 10 Mar 2024 21:02:09 +0000

I modified the test case so the text in the cells have an space, so the extremes are different. A cell can be made smaller until the longes word, and it can be made bigger until all the words are in the same line, at which point there is not need to make it bigger:

```html
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
  <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
  <title>Test table in container with max-width and min-width</title>
    <style type="text/css">
      .main {
        background: lightgreen;
        max-width: 500px;
        min-width: 200px;
        padding: 10px;
      }
      table, th, td {
        padding: 0.25em;
        background: lightblue;
        border: solid 1px black;
      }
  </style>
</head>
<body>
  <div class="main">
    <table width="100%">
      <tr>
        <th>AAA AAA</th>
        <th>BBB BBB</th>
        <th>CCC CCC</th>
      </tr>
      <tr>
        <td>aaa aaa</td>
        <td>bbb bbb</td>
        <td>ccc ccc</td>
      </tr>
    </table>
  </div>
</body>
</html>
```

With the test, the extremes now show the different values 144 and 255 for the table, for the intrinsic values:

![image](https://github.com/dillo-browser/dillo/assets/3866127/eba2fd19-1bbe-40c5-904d-4b10d1995277)

But the minWidth and maxWidth both show 200. The maxWidth should be around 500 pixels, as it is the contentWidth available in the container div.

--%--
From: rodarima
Date: Sun, 10 Mar 2024 22:53:31 +0000

The problem is caused by [this correctRequisition()](https://github.com/dillo-browser/dillo/blob/d61bf779f41617bbc31c3c5697e9275a6fbb1bcd/dw/table.cc#L124), which sets the witdh to 200 instead of leaving the correct value in 500. Adding a simple patch to comment the correction leads to a proper table:

![kk](https://github.com/dillo-browser/dillo/assets/3866127/17cbb1fa-5cd0-460d-95cb-b25cb8795aa4)

I need to investigate what is causing the correctRequisition() call to reduce the width to 200, as it should be left as is.

--%--
From: rodarima
Date: Sun, 10 Mar 2024 22:58:50 +0000

This also solves (one of) the layouting issue(s) of Hacker News:

![kk](https://github.com/dillo-browser/dillo/assets/3866127/0e4f6b25-286a-4122-afe5-2e0349d2168f)

Which had a table too small:

![image](https://github.com/dillo-browser/dillo/assets/3866127/14037099-fe6b-43f0-944b-6899e6336b84)


--%--
From: rodarima
Date: Sun, 17 Mar 2024 09:32:27 +0000

The issue was far more complicated than just the requisition being corrected to a smaller value. I needed to develop a mechanism to instrument calls to see what was going on:

![kk](https://github.com/dillo-browser/dillo/assets/3866127/93920782-dfa8-4083-b286-123bdf2c2f38)

Basically, there are two tasks to perform:

- Compute the available with of the table so it can size the columns.
- Return the size of the table.

The first task was using an incorrect width, as the Widget and Textblock logic handle the case when the CSS width is set to auto in a different way. In this branch, the min-width and max-width of CSS were not used.

The second problem is that when the available width is queried for a widget of which the width is set to auto, the min-width was setting the width first, not expanding the value to max-width in Widget::calcFinalWidth().

Fixing these two issues solves the problem.

--%--
From: rodarima
Date: Sun, 17 Mar 2024 18:40:13 +0000

The requisition correction is breaking Hacker News, but not this particular test, so let's handle that in another issue.