Closed Bug 119100 Opened 23 years ago Closed 21 years ago

width on <td> should be content-box width not border-box width

Categories

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

x86
Windows 2000
defect

Tracking

()

RESOLVED FIXED
mozilla1.4alpha

People

(Reporter: karnaze, Assigned: bernd_mozilla)

Details

(Keywords: testcase)

Attachments

(4 files)

When a width is specified on a <td> it should be the content width. I'm fairly 
sure that it used to work correctly.
Attached file test case
Status: NEW → ASSIGNED
Keywords: testcase
Target Milestone: --- → mozilla0.9.9
Target Milestone: mozilla0.9.9 → mozilla1.0
Moving to Moz1.1
Target Milestone: mozilla1.0 → mozilla1.1
how did I miss this one???
Assignee: karnaze → bernd_mozilla
Status: ASSIGNED → NEW
Priority: -- → P1
Target Milestone: mozilla1.1alpha → mozilla1.4alpha
Attached patch patch rev0Splinter Review
Comment on attachment 116786 [details] [diff] [review]
patch rev0

I runned the regression tests, I had only 80 failures, that I inspected
visually. It looked good to me. We are much more compatible with IE and opera
with this patch. But not to much compatible, as one might see from attachment
#116774 [details]
Attachment #116786 - Flags: superreview?(bzbarsky)
Attachment #116786 - Flags: review?(jkeiser)
Comment on attachment 116786 [details] [diff] [review]
patch rev0

+	   colWidths[colX] = nsTableFrame::RoundToPixel(cellWidth / colSpan,
p2t);

Shouldn't that use (float)colSpan just like the chunk below it does?

Other than that, I just wish we had a good way not to repeat this code all
over... Any way we could refactor it?

sr=bzbarsky in any case.... but it would be good to clean this stuff up a bit.
Attachment #116786 - Flags: superreview?(bzbarsky) → superreview+
Comment on attachment 116786 [details] [diff] [review]
patch rev0

I think it'd be worth creating a GetBorderPadding() method now that nearly
every call to GetPadding() adds in the border.	You can do this here or in
another bug, your choice.
Comment on attachment 116786 [details] [diff] [review]
patch rev0

I think it'd be worth creating a GetBorderPadding() method now that nearly
every call to GetPadding() adds in the border.	You can do this here or in
another bug, your choice.
Attachment #116786 - Flags: review?(jkeiser) → review+
Attached patch revised patchSplinter Review
Comment on attachment 118027 [details] [diff] [review]
revised patch

John and Boris could you please reevaluate your r/sr. Its the the same patch
only some more code refactoring.
Attachment #118027 - Flags: superreview?(bzbarsky)
Attachment #118027 - Flags: review?(jkeiser)
Comment on attachment 118027 [details] [diff] [review]
revised patch

awesome.  ;)
Attachment #118027 - Flags: superreview?(bzbarsky) → superreview+
Comment on attachment 118027 [details] [diff] [review]
revised patch

Aye, happy about that I am :)
Attachment #118027 - Flags: review?(jkeiser) → review+
fix checked in
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: