Last Comment Bug 260406 - cellpadding mapping fails if table cells have other display types
: cellpadding mapping fails if table cells have other display types
Status: RESOLVED FIXED
: testcase
Product: Core
Classification: Components
Component: Layout: Tables (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
:
Mentors:
http://www.data-to-go.com
Depends on: 211636
Blocks: 186317
  Show dependency treegraph
 
Reported: 2004-09-19 13:59 PDT by Julian
Modified: 2011-08-21 22:43 PDT (History)
6 users (show)
bernd_mozilla: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
screenshot (58.57 KB, image/png)
2004-10-08 05:52 PDT, Uri Bernstein (Google)
no flags Details
gif for testcase (20x18) (954 bytes, image/gif)
2004-10-08 06:51 PDT, Uri Bernstein (Google)
no flags Details
testcase (475 bytes, text/html)
2004-10-08 06:57 PDT, Uri Bernstein (Google)
no flags Details

Description Julian 2004-09-19 13:59:13 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Win98; rv:1.7.3) Gecko/20040913 Firefox/0.10
Build Identifier: Mozilla/5.0 (Windows; U; Win98; rv:1.7.3) Gecko/20040913 Firefox/0.10

In a table that uses style sheet images there are some strange artefacts - see
home page at above url.

Reproducible: Always
Steps to Reproduce:
1.
2.
3.



Expected Results:  
Render table cells without any gaps between or artefacts
Comment 1 Boris Zbarsky [:bz] (still a bit busy) 2004-09-21 17:39:33 PDT
Please describe exactly what these artifacts are (post a screenshot?).  The page
looks fine here with a current Linux trunk build.
Comment 2 Uri Bernstein (Google) 2004-10-08 05:52:20 PDT
Created attachment 161469 [details]
screenshot

Artifacts circled in red. I assume this is what the reporter was talking about.
Comment 3 Uri Bernstein (Google) 2004-10-08 06:51:12 PDT
Created attachment 161479 [details]
gif for testcase (20x18)
Comment 4 Uri Bernstein (Google) 2004-10-08 06:57:57 PDT
Created attachment 161480 [details]
testcase

Notice the dark-gray line to the right of the image, indicating that the image
is wrapping, even though the TD width is specified as "20px" (equal to the
width of the gif itself).
The funny thing is that the first table (containing the "P") is necessary:
removing it (or changing its properties) causes the dark line on the bottom
table to disappear.

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.3) Gecko/20041007
Firefox/0.10
Comment 5 Uri Bernstein (Google) 2004-10-08 07:00:45 PDT
All/All, "Layout: Tables", and confirming. Seems valid although might be a
duplicate.
Comment 6 Uri Bernstein (Google) 2004-10-08 12:22:18 PDT
Seeing this also with trunk build:
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a5) Gecko/20041008
Firefox/0.9.1+
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2004-10-09 11:27:20 PDT
OK, the reason this is happening is that the attribute mapping function on table
cells maps data differently depending on something other than attributes (to
whit, the computed display of the table cell).  See dbaron's comment in
nsHTMLTableCellElement::WalkContentStyleRules.

So in this case, we don't map padding from the table's cellpadding attribute in
the first table (because the table cell is not display:table-cell).  Then we
cache the padding struct at that spot in the ruletree.

When the second cell goes to resolve style, we have a struct cached in the
ruletree already (because a given rule should always give the same style data),
so we don't even call WalkContentStyleRules().  The problem, of course, is that
this rule _does_ give different data depending on how it's asked... which is
what the comment is about.

See also the XXX comment at the beginning of MapAttributesIntoRule() in
nsHTMLTableElement.

In short, we need to be checking the tag, not the display type.  The problem is
finding out what node we're dealing with...
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2004-10-10 13:27:24 PDT
I've been thinking about this a bit, and the best I've come up with so far is as
follows:

1)  Make it possible to get the mMappedAttributes off the nsAttrAndChildArray
2)  Create another nsIStyleRule impl that will have a pointer to the table's
    mapped attrs and get stuff from them as needed when it's asked to compute
    style data.
3)  When SetAttr() happens on the table, dump the old nsIStyleRule if the
    mMappedAttributes changed.
4)  Table cells pass this new nsIStyleRule (which they have to be able to get
    off the table) to the rulewalker instead of passing the table's mapped
    attributes.

Does that seem reasonable?  Step 1 involves exposing some innards of
nsAttrAndChildArray and Step 4 may end up involving a cast to nsHTMLTableElement
or something....
Comment 9 Jonas Sicking (:sicking) No longer reading bugmail consistently 2004-10-11 02:45:55 PDT
mMappedAttrs already is an nsIStyleRule, would this rule do anything different
then what the current one does?
Comment 10 Boris Zbarsky [:bz] (still a bit busy) 2004-10-11 07:39:17 PDT
> would this rule do anything different then what the current one does?

Absolutely.  What the current rule does is very very wrong (see comment 7).

In brief, this other rule would just use a different attr-mapping function,
called on the same mapped attrs.
Comment 11 Bernd 2011-08-21 22:43:42 PDT
fixed by the checkin fro bug 211636 http://hg.mozilla.org/mozilla-central/rev/4738b38a2f3c

Note You need to log in before you can comment on or make changes to this bug.