Bug 1534334 Comment 2 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Patrick Brosset <:pbro> from comment #1)
> Would this code have any chance of actually working in most use cases you think?

I can think of a few issues:
 (1) RE the geometry here: You're using getBoxQuads, but that doesn't reliably report the area of the box that could create scrollable overflow. It lets you pick content/padding/border/margin-box, but unfortunately, we don't consistently use the same one of those boxes for scrollable-overflow determination. E.g. this testcase:
 https://bugzilla.mozilla.org/attachment.cgi?id=9043507
...has a 100px margin below each of the green boxes (inside each scrollable area), but we only consider that margin to be "scrollable" in the first case there. (Chrome disagrees and considers it scrollable in all three cases. The spec says that margins are optionally includable in scrollable overflow: https://drafts.csswg.org/css-overflow-3/#scrollable )

 So: bottom-line, it's ~hard to predict which getBoxQuads box you should be using in this JS code.

 (2) RE the rule-reporting: I'm not sure it makes sense to use `display:contents` to find a "guilty" report-worthy rule... Suppose we have e.g. these rules applying to some overflowing `<div class="a">`:
```
 .a { width: 20000px;
      /* more stuff */ }
 .a { padding-left: 99999px;
      /* more stuff */ }
 .a { display: inline-block }
```

I would think we'd **want** to blame the first and/or second rules here, because they each make the div super-wide.

But from my understanding, your approach would report the third rule as "guilty", because that's the only one where setting `display` will have any effect.  (If you add `display:contents` to any earlier rule, it has zero effect, because it's overridden by the `display` declaration in the third rule, because these rules all have the same specificity and so the last one to set `display` will win.)

> Are there more efficient ways of doing this? Would platform layout code be better equipped to do something like this?

For the "tell me which elements have boxes that are in the overflow area" part: I think platform layout code would be best equipped to do something like this.

For the "tell me which rules cause those elements to overflow": that is a hard problem, which I don't think has an easy answer even with perfect information. Concerns:
 * E.g. suppose you have a div with N different CSS rules, each of which contribute to its size (setting padding/border/height/width all in different rules, and another rule that sets `transform: scale(1.2)` to make it slightly bigger.  Suppose that removing any one of these rules is sufficient to avoid overflow. How do we decide which rule(s) to blame?

 * In some (many?) cases, it's not a rule but really the **content** that's to blame, not any CSS rule. E.g. if I have a `<img>` with a 1000px tall PNG, with `img { border: 2px solid black }`.  If this barely overflows its scrollport (say, by 2px) and we have to decide which rule to blame, would it make sense to blame the border?  If the author removes that and then later swaps in a different PNG that is 1024px tall, then it would overflow again, so the border wasn't really the thing "most at fault" -- it was the content-size, really.

It might be possible to come up with a "blame" system but we'd need to work through the edge cases pretty thoroughly and come up with what makes sense to report in "shared blame" scenarios like this, I think.
(In reply to Patrick Brosset <:pbro> from comment #1)
> Would this code have any chance of actually working in most use cases you think?

I can think of a few issues:
 (1) RE the geometry here: You're using getBoxQuads, but that doesn't reliably report the area of the box that could create scrollable overflow. It lets you pick content/padding/border/margin-box, but unfortunately, we don't consistently use the same one of those boxes for scrollable-overflow determination. E.g. this testcase:
 https://bugzilla.mozilla.org/attachment.cgi?id=9043507
...has a 100px margin below each of the green boxes (inside each scrollable area), but we only consider that margin to be "scrollable" in the first case there. Chrome disagrees and considers it scrollable in all three cases. The spec says that margins are **optionally** includable in scrollable overflow: https://drafts.csswg.org/css-overflow-3/#scrollable So for now, there's no right answer and no great way for you to know which box to use. :(

 So: bottom-line, it's ~hard to predict which getBoxQuads box you should be using in this JS code.

 (2) RE the rule-reporting: I'm not sure it makes sense to use `display:contents` to find a "guilty" report-worthy rule... Suppose we have e.g. these rules applying to some overflowing `<div class="a">`:
```
 .a { width: 20000px;
      /* more stuff */ }
 .a { padding-left: 99999px;
      /* more stuff */ }
 .a { display: inline-block }
```

I would think we'd **want** to blame the first and/or second rules here, because they each make the div super-wide.

But from my understanding, your approach would report the third rule as "guilty", because that's the only one where setting `display` will have any effect.  (If you add `display:contents` to any earlier rule, it has zero effect, because it's overridden by the `display` declaration in the third rule, because these rules all have the same specificity and so the last one to set `display` will win.)

> Are there more efficient ways of doing this? Would platform layout code be better equipped to do something like this?

For the "tell me which elements have boxes that are in the overflow area" part: I think platform layout code would be best equipped to do something like this.

For the "tell me which rules cause those elements to overflow": that is a hard problem, which I don't think has an easy answer even with perfect information. Concerns:
 * E.g. suppose you have a div with N different CSS rules, each of which contribute to its size (setting padding/border/height/width all in different rules, and another rule that sets `transform: scale(1.2)` to make it slightly bigger.  Suppose that removing any one of these rules is sufficient to avoid overflow. How do we decide which rule(s) to blame?

 * In some (many?) cases, it's not a rule but really the **content** that's to blame, not any CSS rule. E.g. if I have a `<img>` with a 1000px tall PNG, with `img { border: 2px solid black }`.  If this barely overflows its scrollport (say, by 2px) and we have to decide which rule to blame, would it make sense to blame the border?  If the author removes that and then later swaps in a different PNG that is 1024px tall, then it would overflow again, so the border wasn't really the thing "most at fault" -- it was the content-size, really.

It might be possible to come up with a "blame" system but we'd need to work through the edge cases pretty thoroughly and come up with what makes sense to report in "shared blame" scenarios like this, I think.
(In reply to Patrick Brosset <:pbro> from comment #1)
> Would this code have any chance of actually working in most use cases you think?

I can think of a few issues:
 (1) RE the geometry here: You're using getBoxQuads, but that doesn't reliably report the area of the box that could create scrollable overflow. It lets you pick content/padding/border/margin-box, but unfortunately, we don't consistently use the same one of those boxes for scrollable-overflow determination. E.g. this testcase:
 https://bugzilla.mozilla.org/attachment.cgi?id=9043507
...has a 100px margin below each of the green boxes (inside each scrollable area), but we only consider that margin to be "scrollable" in the first case there. Chrome disagrees and considers it scrollable in all three cases. The spec says that margins are **optionally** includable in scrollable overflow: https://drafts.csswg.org/css-overflow-3/#scrollable So for now, there's no right answer and no great way for you to know which box to use. :(

 So: bottom-line, it's ~hard to predict which getBoxQuads box you should be using in this JS code.

 (2) RE the rule-reporting: I'm not sure it makes sense to use `display:contents` to find a "guilty" report-worthy rule... Suppose we have e.g. these rules applying to some overflowing `<div class="a">`:
```
 .a { width: 20000px;
      /* more stuff */ }
 .a { padding-left: 99999px;
      /* more stuff */ }
 .a { display: inline-block }
```

I would think we'd **want** to blame the first and/or second rules here, because they each make the div super-wide.

But from my understanding, your approach would report the third rule as "guilty", because that's the only one where setting `display` will have any effect.  (If you add `display:contents` to any earlier rule, it has zero effect, because it's overridden by the `display` declaration in the third rule, because these rules all have the same specificity and so the last one to set `display` will win.)

> Are there more efficient ways of doing this? Would platform layout code be better equipped to do something like this?

For the "tell me which elements have boxes that are in the overflow area" part: I think platform layout code would be best equipped to do something like this.

For the "tell me which rules cause those elements to overflow": that is a hard problem, which I don't think has an easy answer even with perfect information. Concerns:
 * E.g. suppose you have a div with N different CSS rules, each of which contribute to its size -- e.g. different rules setting padding/border/height/width, and another rule that sets `transform: scale(1.2)` to make it slightly bigger.  And maybe another rule that sets `aspect-ratio` once we eventually support that property. Now, suppose that this box just barely overflows, and removing *any one* of these rules is sufficient to avoid overflow. How do we decide which rule(s) to blame?

 * In some (many?) cases, it's not a rule but really the **content** that's to blame, not any CSS rule. E.g. if I have a `<img>` with a 1000px tall PNG, with `img { border: 2px solid black }`.  If this barely overflows its scrollport (say, by 2px) and we have to decide which rule to blame, would it make sense to blame the border?  If the author removes that and then later swaps in a different PNG that is 1024px tall, then it would overflow again, so the border wasn't really the thing "most at fault" -- it was the content-size, really.

It might be possible to come up with a "blame" system but we'd need to work through the edge cases pretty thoroughly and come up with what makes sense to report in "shared blame" scenarios like this, I think.
(In reply to Patrick Brosset <:pbro> from comment #1)
> Would this code have any chance of actually working in most use cases you think?

I can think of a few issues:
 (1) RE the geometry here: You're using getBoxQuads, but that doesn't reliably report the area of the box that could create scrollable overflow. It lets you pick content/padding/border/margin-box, but unfortunately, we don't consistently use the same one of those boxes for scrollable-overflow determination. E.g. this testcase:
 https://bugzilla.mozilla.org/attachment.cgi?id=9043507
...has a 100px margin below each of the green boxes (inside each scrollable area), but we only consider that margin to be "scrollable" in the first case there. Chrome disagrees and considers it scrollable in all three cases. The spec says that margins are **optionally** includable in scrollable overflow: https://drafts.csswg.org/css-overflow-3/#scrollable So for now, there's no right answer and no great way for you to know which box to use. :(

 So: bottom-line, it's ~hard to predict which getBoxQuads box you should be using in this JS code.

 (2) RE the rule-reporting: I'm not sure it makes sense to use `display:contents` to find a "guilty" report-worthy rule... Suppose we have e.g. these rules applying to some overflowing `<div class="a">`:
```
 .a { width: 20000px;
      /* more stuff */ }
 .a { padding-left: 99999px;
      /* more stuff */ }
 .a { display: inline-block }
```

I would think we'd **want** to blame the first and/or second rules here, because they each make the div super-wide.

But from my understanding, your approach would report the third rule as "guilty", because that's the only one where setting `display` will have any effect.  (If you add `display:contents` to any earlier rule, it has zero effect, because it's overridden by the `display` declaration in the third rule, because these rules all have the same specificity and so the last one to set `display` will win.)

> Are there more efficient ways of doing this? Would platform layout code be better equipped to do something like this?

For the "tell me which elements have boxes that are in the overflow area" part: I think platform layout code would be best equipped to do something like this.

For the "tell me which rules cause those elements to overflow": that is a hard problem, which I don't think has an easy answer even with perfect information. Concerns:
 * E.g. suppose you have a div with N different CSS rules, each of which contribute to its size -- e.g. different rules setting padding/border/height/width, and another rule that sets `transform: scale(1.2)` to make it slightly bigger.  And maybe another rule that sets `aspect-ratio` once we eventually support that property. Now, suppose that this box just barely overflows, and removing *any one* of these rules is sufficient to avoid overflow. How do we decide which rule(s) to blame?

 * In some (many?) cases, it's really the **content** that's to blame, not any CSS rule. E.g. suppose I have a `<img>` with a 1000px tall PNG, with `img { border: 2px solid black }`.  If this barely overflows its scrollport (say, by 2px) and we have to decide which rule to blame, would it make sense to blame the border?  If the author removes that and then later swaps in a different PNG that is 1024px tall, then it would overflow again, so the border wasn't really the thing "most at fault" -- it was the content-size, really.

It might be possible to come up with a "blame" system but we'd need to work through the edge cases pretty thoroughly and come up with what makes sense to report in "shared blame" scenarios like this, I think.

Back to Bug 1534334 Comment 2