1508921 - BCCornerInfo::Set() truncates 4-bit BCBorderOwner enum value to 3 bits to work around frame_above_rules_all.html test failure

Status: NEW

(Core :: Layout: Tables, defect, P3)

Description

[Chris Peterson [:cpeterson]]

Attachment: frame_above_rules_all.png

The assignment at [1] truncates a BCBorderOwner enum value ([2], which can hold values 0-10, thus requiring at least 4 bits) to BCCornerInfo's ownerElem member variable ([3], a 3-bit bit field that can only hold up values 0-7). Thus BCCornerInfo's ownerElem cannot hold BCBorderOwner enum values eAjaRowOwner, eCellOwner, or eAjaCellOwner.

In bug 1485179, I resized BCCornerInfo's ownerElem and subElem bit fields to fit all BCBorderOwner enum values. Unfortunately, copying all the bits of the BCBorderOwner enum value causes mochitest layout/reftests/table-bordercollapse/frame_above_rules_all.html to fail [4] because some border corner pixels are not drawn.

See the attached screenshot for a comparison of the frame_above_rules_all.html and frame_above_rules_all_ref.html test files rendered side-by-side in Firefox, Chrome, and Edge. 

* In Firefox, both test files are missing border pixels at bottom right corners of "Row 1, Cell 2" and "Row 2, Cell 1".

* In Chrome, there are no missing pixels, but the borders are grey in frame_above_rules_all.html and black in frame_above_rules_all_ref.html.

* In Edge, both test files look correct.

The missing corner pixels are actually a regression from device pixel scaling bug 895096 in Firefox 57, which just happened to cause the same missing corner pixels in both the frame_above_rules_all.html and frame_above_rules_all_ref.html test files. With the bit field fix from bug 1485179 applied, the results changed slightly.

[1] https://searchfox.org/mozilla-central/rev/55895c49f55073d82d977cb74ec1d3a71ae4b25f/layout/tables/nsTableFrame.cpp#5318
[2] https://searchfox.org/mozilla-central/rev/55895c49f55073d82d977cb74ec1d3a71ae4b25f/layout/tables/celldata.h#131-145
[3] https://searchfox.org/mozilla-central/rev/55895c49f55073d82d977cb74ec1d3a71ae4b25f/layout/tables/nsTableFrame.cpp#5303
[4] https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://queue.taskcluster.net/v1/task/R_rkPPacSECjwKaAPB27SA/runs/1/artifacts/public/logs/live_backing.log&only_show_unexpected=1&only_show_unexpected=1

Comment 1

[Chris Peterson [:cpeterson]]

Attachment: frame_above_rules_all_modified.html

frame_above_rules_all_modified.html is a minimized test case that amplifies the border corner bug from frame_above_rules_all.html.

The frame_above_rules_all_modified.html test case is rendered differently in Firefox, Chrome, and Edge. The green dashed border style show some interesting spacing differences between Chrome and Safari.

The red border height problem has something to do with colspan=2 when top row is in <thead> and the colspan=2 bottom row is in <tbody>. Moving the two rows both into <thead> or <tbody> makes the red border height problem go away.

BCBlockDirSeg::GetBEndCorner() has cornerSubWidth=0 in the short case (in the middle of the colspan=2) and cornerSubWidth=54 in the good case.

Comment 2

[Chris Peterson [:cpeterson]]

Attachment: Screenshot.png

For posterity, here is a screenshot of the attached frame_above_rules_all_modified.html test case loaded side-by-side in Firefox 64, Chrome 72, and Edge 18 on Windows 10. Note the differences in border overlapping and dash spacing.