Closed Bug 45621 Opened 25 years ago Closed 24 years ago

Both <td nowrap> and css-style:nowrap blocks relative width on childs

Categories

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

Product:
Core
Component:
Layout: Tables
Platform:
x86
Windows NT
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.0

People

(Reporter: make, Assigned: alexsavulov)

References

Details

(Keywords: testcase)

Attachments

(4 files, 3 obsolete files)

testcase
1.07 KB, text/html
Details
Another testcase (from bug 59448)
223 bytes, text/html
Details
test with input and image - regression test file
912 bytes, text/html
Details
pulled GetFlag(BRS_NOWRAP) out of the loop
4.05 KB, patch
alexsavulov
: review+
alexsavulov
: superreview+
Details | Diff | Splinter Review
From Bugzilla Helper: User-Agent: Mozilla/4.73 [en] (WinNT; U) BuildID: 2000071608 Cells with either <td nowrap> or with the css style white-space:nowrap will block proper width calculations of cell contents. Reproducible: Always Steps to Reproduce: 1. Load the attached testcase Actual Results: Relative width of form input not applied. Expected Results: Relative width calculation should work.
Keywords: testcase
Attached file testcase —
This is similar to bug 41306 in some ways. Nowrap triggers 1-pass reflow, I assume. Should this work?
QA Contact: desale → chrisd
*** Bug 59448 has been marked as a duplicate of this bug. ***
*** Bug 59448 has been marked as a duplicate of this bug. ***
Marc, we need to remove the nowrap logic from tables and make the block code handle them correctly. Moving to m1.0
Assignee: karnaze → attinasi
Target Milestone: --- → mozilla1.0
QA contact update
QA Contact: chrisd → amar
Status: NEW → ASSIGNED
Blocks: 85276
No longer blocks: 85276
Reassigning to alexsavulov.
Assignee: attinasi → alexsavulov
Status: ASSIGNED → NEW
<hr> not reflowed correctly even after aplying the patch that fixes bug 80817 (attachment 44612 [details] [diff] [review]). Enlarge/shrink the default text size to see what happens with <hr>. Now either we should wait with that patch or aply it and find the reason for this malfunction here.
type some text in the first two inputs - they change imediately their width (I think that Mark Attinasi had some similar bug)
Blocks: 95268
Comment on attachment 52153 [details] test with input and image - regression test file ><!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd"><html><head><title>Untitled</title></head> ><body> > <b>the table below contains two cells in two rows:<br> the first one has an input in it (width=100%)<br> the second one has an image (width=100% , height=40px)</b> > <table style="width: 50%; background-color: silver; border-top-width: 0px; border-top-style: solid; border-top-color: red; border-right-width: 0px; border-right-style: solid; border-right-color: red; border-bottom-width: 0px; border-bottom-style: solid; border-bottom-color: red; border-left-width: 0px; border-left-style: solid; border-left-color: red; -x-border-x-spacing: 0px; -x-border-y-spacing: 0px; border-collapse: collapse; "> > <tbody><tr> > <td style="white-space: nowrap; background-color: gray; padding-top: 0px; padding-right: 0px; padding-bottom: 0px; padding-left: 0px; border-top-width: 0px; border-top-style: solid; border-top-color: black; border-right-width: 0px; border-right-style: solid; border-right-color: black; border-bottom-width: 0px; border-bottom-style: solid; border-bottom-color: black; border-left-width: 0px; border-left-style: solid; border-left-color: black; "> > <input type="text" style="width: 100%; background-color: pink; border-top-width: 0px; border-top-style: solid; border-top-color: green; border-right-width: 0px; border-right-style: solid; border-right-color: green; border-bottom-width: 0px; border-bottom-style: solid; border-bottom-color: green; border-left-width: 0px; border-left-style: solid; border-left-color: green; "/> > </td> > </tr> > <tr> > <td style="white-space: nowrap; background-color: gray; padding-top: 0px; padding-right: 0px; padding-bottom: 0px; padding-left: 0px; border-top-width: 0px; border-top-style: solid; border-top-color: black; border-right-width: 0px; border-right-style: solid; border-right-color: black; border-bottom-width: 0px; border-bottom-style: solid; border-bottom-color: black; border-left-width: 0px; border-left-style: solid; border-left-color: black; "> > <img style="width: 100%; height: 40px; background-color: cyan; border-top-width: 0px; border-top-style: solid; border-top-color: green; border-right-width: 0px; border-right-style: solid; border-right-color: green; border-bottom-width: 0px; border-bottom-style: solid; border-bottom-color: green; border-left-width: 0px; border-left-style: solid; border-left-color: green; "/> > </td> > </tr> > </tbody></table> > ></body></html>
Attached patch proposed patch - need sr=/r= (obsolete) — — Splinter Review
Comment on attachment 52162 [details] [diff] [review] proposed patch - need sr=/r= r=dbaron
Attachment #52162 - Flags: review+
Although actually it would be preferable (slightly faster and smaller code) if you consolidated the logic to look something like this: // We let child blocks make their own decisions the same // way we are here. // // For inline lines with no-wrap, the only way things // could change is if there is a percentage-sized child. if (line->IsBlock() || line->HasPercentageChild() || (!notWrapping && ((line->mNext && !line->HasBreak()) || line->ResizeReflowOptimizationDisabled || line->HasFloaters() || line->IsImpactedByFloater() || (line->mBounds.XMost() > newAvailWidth)))) { line->MarkDirty(); }
David: I agree, i could also reduce that "notWrapping", On the other side I have to take care of that debug part: #ifdef REALLY_NOISY_REFLOW ... also the debug part that follows #ifdef DEBUG ... is to be changed. Is using both notWrapping and aState.GetFlag(BRS_NOWRAP) What about this: while (nsnull != line) { // We let child blocks make their own decisions the same // way we are here. // // For inline lines with no-wrap, the only way things // could change is if there is a percentage-sized child. if (line->IsBlock() || line->HasPercentageChild() || (!aState.GetFlag(BRS_NOWRAP) && ((line->mNext && !line->HasBreak()) || line->ResizeReflowOptimizationDisabled || line->HasFloaters() || line->IsImpactedByFloater() || (line->mBounds.XMost() > newAvailWidth)))) { line->MarkDirty(); } #ifdef REALLY_NOISY_REFLOW if (!line->IsBlock()) { printf("PrepareResizeReflow thinks line %p is %simpacted by floaters\n", line, line->IsImpactedByFloater() ? "" : "not "); } #endif #ifdef DEBUG if (gNoisyReflow && !line->IsDirty() && !aState.GetFlag(BRS_NOWRAP)) { IndentBy(stdout, gNoiseIndent + 1); printf("skipped: line=%p next=%p %s %s %s%s%s breakType=%d xmost=%d\n", line, line->mNext, line->IsBlock() ? "block" : "inline", "no-wrap", line->HasBreak() ? "has-break " : "", line->HasFloaters() ? "has-floaters " : "", line->IsImpactedByFloater() ? "impacted " : "", line->GetBreakType(), line->mBounds.XMost()); } #endif } line = line->mNext; }
small mistake: instead of "no-wrap" comes "wrapping"
Attachment #52162 - Attachment is obsolete: true
The indentation there looks really funky. Are you using tabs? Please don't use tabs. Also, having the notWrapping invariant pulled out of the loop is probably a good thing.
the problem... there were some tabs before in the source code... i will remove them all when i check in the code
Comment on attachment 52301 [details] [diff] [review] same patch but the logic is more consolidated replaced with next patch file
Attachment #52301 - Attachment is obsolete: true
Comment on attachment 52551 [details] [diff] [review] replaces previous patch - tabs replaced with spaces (VS was reinstalled on my machine, such a pain) sr=attinasi
Attachment #52551 - Flags: superreview+
Comment on attachment 52551 [details] [diff] [review] replaces previous patch - tabs replaced with spaces (VS was reinstalled on my machine, such a pain) r=dbaron if you pull the |notWrapping| variable back out of the loop
Attachment #52551 - Flags: review+
Is it really any faster to check a variable vs. calling GetFlag() in the loop?
A few instructions. But I guess it's ok if you don't, too...
Comment on attachment 52551 [details] [diff] [review] replaces previous patch - tabs replaced with spaces (VS was reinstalled on my machine, such a pain) replaced by attachment 52573 [details] [diff] [review]
Attachment #52551 - Attachment is obsolete: true
Comment on attachment 52573 [details] [diff] [review] pulled GetFlag(BRS_NOWRAP) out of the loop from attachment 55251 [details]
Attachment #52573 - Flags: superreview+
Attachment #52573 - Flags: review+
fixed on trunk
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
verified fixed. Build ID: 2002011803. WIN2K
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: