302959 - If the a element in the legend element got focus, the ancestor table width is broken

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

testcase:
http://bugzilla.mozilla.gr.jp/attachment.cgi?id=2871&action=view

See testcase, and set focus to anchor element in legend element.
The ancestor table element(or td element) is shrunk to minimum fieldset elements
width.

This is not reproduced on Firefox 1.0.x.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.0

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 191212 [details] [diff] [review]
Patch rv1.0

max-element-width is normally less than width (and never greater).  Since
fieldsets can vary in width, this patch is incorrect.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Also note that the testcase doesn't even have anything to do with
max-element-width (also known as minimum width).  It's a testcase for intrinsic
width, and the only reason this patch fixes anything is that there's a hack in
the table code to ensure that the intrinsic width is never less than the
max-element-width by increasing the intrinsic width.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv2.0

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 191219 [details] [diff] [review]
Patch rv2.0

What says contentRect is right?

I think you need something more like the old code, except that:
 * you need to ensure that you reflow both the legend and the contents when
NS_REFLOW_CALC_MAX_WIDTH is set
 * you need to consider the legend too

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv3.0 -u8pw

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv3.0 for check-in

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

NS_REFLOW_CALC_MAX_WIDTH could be set with a reason other than incremental, I'd
think.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv3.1

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 191227 [details] [diff] [review]
Patch rv3.1

Thank you, David!

The risk is low. This is regression from 1.0.x.

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

checked-in. Thank you.