Closed Bug 242253 Opened 20 years ago Closed 20 years ago

text in table cells not painted when visibility goes from collapsed to visible

Categories

(Core :: Layout: Tables, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bernd_mozilla, Assigned: bernd_mozilla)

Details

(Keywords: testcase)

Attachments

(4 files, 2 obsolete files)

a the row visibility toggles from collapsed to visible the text is not redrawn.

fantasai, do you have an idea where that happens
Attached file testcase
this became visible after my checkin for bug 77019
Attached patch patch (obsolete) — Splinter Review
Comment on attachment 147486 [details] [diff] [review]
patch

I need the change in the argument to  AdjustForCollapsingCols and
AdjustForCollapsingRows for the next step, hooking up the overflow handling
Attachment #147486 - Flags: superreview?(dbaron)
Attachment #147486 - Flags: review?(dbaron)
taking, I believe the patch fixes all issues in attachment 147353 [details] from bug 77019
Assignee: nobody → bernd_mozilla
Comment on attachment 147486 [details] [diff] [review]
patch

>+        while (cellFrame) {
>+          const nsStyleDisplay* cellDisplay = cellFrame->GetStyleDisplay();
>+          if (NS_STYLE_DISPLAY_TABLE_CELL == cellDisplay->mDisplay) {
>+            nsTableCellFrame* cFrame = (nsTableCellFrame*)cellFrame;
>+            nsPoint offset;
>+            cFrame->GetCollapseOffset(aPresContext, offset);
>+            if (offset.y)
>+              cFrame->SetCollapseOffsetY(aPresContext, 0);

Why not just cut everything from "nsPoint offset" to "if (offset.y)" and change
nsTableCellFrame::SetCollapseOffset{X,Y} to pass |aOffsetX != 0| instead of
|PR_TRUE| for the last argument?
Comment on attachment 147486 [details] [diff] [review]
patch

>+              cFrame->SetCollapseOffsetY(aPresContext, 0);

Also, why is this 0 and not -aYGroupOffset?
Attached file Global testcase (obsolete) —
please notice that the global testcase invokes the border collapse code and the
bc border painting is broken.
Attached patch revised patchSplinter Review
Thanks David for that nifty code in SetCollapseOffset.

>Also, why is this 0 and not -aYGroupOffset?

Because the uncollapsed position is 0 see
http://lxr.mozilla.org/seamonkey/source/layout/html/table/src/nsTableCellFrame.cpp#421
ff

The idea is that the cell content of rowspanning cells is moved up. The cells
are moved relative to the row. So if a row is not collapsed we dont need to
move relative to the row.

The comment in nsTableCellFrame.cpp leaves however a big question mark how
correct the whole approach is. 

I dont read from CSS2.1:

The 'visibility' property takes the value 'collapse' for row, row group,
column, and column group elements. This value causes the entire row or column
to be removed from the display, and the space normally taken up by the row or
column to be made available for other content. Contents of spanned rows and
columns that intersect the collapsed column or row are clipped. The suppression
of the row or column, however, does not otherwise affect the layout of the
table. This allows dynamic effects to remove table rows or columns without
forcing a re-layout of the table in order to account for the potential change
in column constraints.

that cells that originate in a row and are rowspanning should be displayed if
the originating row is collapsed. I see only that cells that span into that row
should be shortened.

The whole code is pretty old and did previously only get executed on the
initial reflow. It lacks currently any overflow reporting. I would prefer to go
with that reset to 0 now and fix or even better toremove that shift buisiness
when reporting the correct overflow areas.
Attachment #147486 - Attachment is obsolete: true
Attachment #147916 - Flags: superreview?(dbaron)
Attachment #147916 - Flags: review?(dbaron)
Attachment #147916 - Flags: superreview?(dbaron)
Attachment #147916 - Flags: superreview+
Attachment #147916 - Flags: review?(dbaron)
Attachment #147916 - Flags: review+
Attachment #147842 - Attachment is obsolete: true
fix checked in. I filed bug 242998 for the overflow area and bug 242997 for the
broder collapse issue.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Attachment #147486 - Flags: superreview?(dbaron)
Attachment #147486 - Flags: review?(dbaron)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: