Closed Bug 40828 Opened 24 years ago Closed 23 years ago

cellpadding disappears when changing style with dom, and when changing text size

Categories

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

x86
All
defect

Tracking

()

VERIFIED WORKSFORME

People

(Reporter: hyp-x, Assigned: karnaze)

References

()

Details

(Keywords: css1, testcase, Whiteboard: [rtm-] fix-in-hand a=buster, r=dcone)

Attachments

(5 files)

Version tested:
2000-05-26-20 on Win98SE

Reproduction:
* Load the attached testcase
it contains a table with only one cell
* Change the textsize with any of the following:
View/Enlarge text size, View/Reduce text size, Ctrl+MouseWheel

What happens (and shouldn't):
The cellpadding disappears from the table.

Note:
Changing back the fontsize doesn't gives back the cellpadding.
Attached file Testcase
Keywords: testcase
Whiteboard: fix-in-hand
Status: NEW → ASSIGNED
Target Milestone: --- → M18
I ran across a similar problem with cellpadding disappearing while I was 
splitting bug 51037: an position:absolute table will lose implicit or explicit 
cellpadding when its style.visible is set to "visible".  Does the patch fix 
this problem as well?
*** Bug 54118 has been marked as a duplicate of this bug. ***
Adding keywords from bug 54118 and marking pdt+ (the code reviews should happen 
before PDT sees this). 

PDT, this is low hanging fruit, the fix has been around quite a while, and Ian 
thinks it is very important.
Keywords: correctness, css1, rtm
Whiteboard: fix-in-hand → [rtm+] fix-in-hand
I don't think that you are 100% guaranteed that kidFrame ISA nsTableCellFrame.
For example, nsTableRowFrame has code that guards against the possibility that
it is not:

nsTableRowFrame::CalcTallestCell()
{
  [snip]
  for (nsIFrame* kidFrame = mFrames.FirstChild(); kidFrame;
       kidFrame->GetNextSibling(&kidFrame)) {
    nsCOMPtr<nsIAtom> frameType;
    kidFrame->GetFrameType(getter_AddRefs(frameType));
    if (nsLayoutAtoms::tableCellFrame == frameType.get())
    {
      nscoord availWidth = ((nsTableCellFrame *)kidFrame)->GetPriorAvailWidth();
      etc....

You need to add this kind of check before you do the cast.
Buster, you can't see it in the patch, but the new code is actually inside an if 
statement that checks the frame type.
in that case, a=buster
Whiteboard: [rtm+] fix-in-hand → [rtm+] fix-in-hand a=buster
Priority: P3 → P2
PDT marking [rtm need info] since only one reviewer is listed. Please mark
[rtm+] after peer review.
Whiteboard: [rtm+] fix-in-hand a=buster → [rtm need info] fix-in-hand a=buster
I tried out the patch and it worked fine r=rods
Looks good to me.  r=dcone
Marking rtm+ again.
Whiteboard: [rtm need info] fix-in-hand a=buster → [rtm+] fix-in-hand a=buster
rtm-, wouldn't pull it off the wire for this.
Whiteboard: [rtm+] fix-in-hand a=buster → [rtm-] fix-in-hand a=buster
this strikes me as pretty serious.

#1 - if the user changes their text size the tables lose their padding
#2 - if a script sets visibility on a table (and how DHTML/Layout work isn't 
ina table these days? CSS isn't that widely supported, lets be realistic) then 
the cellpadding is lost.
#3 - if a previously hidden table is shown the table padding is lost.
#4 - if you animate the table (slide it accross the screen or anything) the 
padding is lost. 

bug 56725 *is* looking like a dup ....

i'm confused by "wouldn't pull it off the wire" .... please help a (realively 
speaking) newbie to mozilla out ... ok, so it doesn't crash the browser but it 
does render a VERY high number of any DHTML/DOM scripting painful to the nth 
degree. the only work around i can isolate is toggling the display property, 
but if the item is being animated, you simply can't do that.

this has the potential to impact other things. if the table has lost it's 
padding, the dimensions and the positioning could (as reflected in the scripts) 
be impacted as well.  this can break all kinds of DHTML code.

is it really too late for RTM on this?

Some New Testcases demonstrating all these behaviors are here: 
http://www.cherny.com/mozilla/padding/

This seems like a big deal to my little mind.
Changing rtm- to rtm+ due to low risk. Sorry PDT, but my conscience is bothering 
me. Our simple minds are happy to ship a product, while web developers want 
their pages to look right.
Whiteboard: [rtm-] fix-in-hand a=buster → [rtm+] fix-in-hand a=buster, r=dcone
Whiteboard: [rtm+] fix-in-hand a=buster, r=dcone → [rtm-] fix-in-hand a=buster, r=dcone
PDT still thinks this is rtm- given the existing test cases. If we had an
example of a frequently visited web page which was severely damaged by this bug,
we might reconsider.
>> If we had an example of a frequently visited web page which was severely
>> damaged by this bug, we might reconsider.

Yes, this is bordering on a flame ... My apologies ... it's early and I'm tired.
 But the comment above is bothersome ... and I just spent almost an entire day
explaining to all my coworkers in a presentation how "DHTML IS ALIVE AND WELL IN
MOZILLA" ... well maybe not.

#1 - There *WILL NEVER BE PAGES THAT ARE FREQUENTLY VISITED WITH DHTML* if
Netscape keeps putting out entirely TOO BUGGY A PRODUCT FOR IT TO BE DEPENDED
ON.  So your comment that we need examples of frequently visited pages is a weak
arguement. #2 - There are VERY FEW IF ANY DHTML pages that work in Mozilla yet
anyway because people don't design for beta browsers ... how can that be made an
argument with a technology that isn't supported effectively yet?  So again,
that's a weak argument.

Mozilla WAS SUPPOSED TO BE that solution.

You've been able to effectively do DHTML in INTERNET EXPLORER SINCE 1997!!!.

Developers (I'm one, so are all my coworkers and they can't believe this) can't
build with it cuz both browsers don't effectively support it.  When we do, we
add days and hours to any project just to get it to work in a Netscape Browser.

Mozilla WAS SEEN as our last hope ... now I'm losing confidence.  This was the
Web Standards Project's and developers AROUND THE WORLD'S challenge: Put out a
browser that is as bug free as possible, standards compliant and the behavior of
which can be depended on instead of having broken implementations of certain
features.  Guess what? You're not doing it by not fixing things like this (I
realize it will be fixed later ... but upon the FIRST RELEASE after TWO years is
when reviewers and developers will be watching, NOT 6.01 - they won't care about
patches).  More and more recently in the decisions of what gets fixed by when
I'm seeing the corporate influence of a release date that borders on a Microsoft
like decision -- let the people who use the product be our beta testers, never
mind that it is a release version ... we'll issue a service patch anyway.  So
make life harder for developers, they'll have to branch their scripts again and
do a detection for version 6.0 vs. 6.01 ... [exasperated sigh]

Web builders build pages and we're constantly having to explain to our project
managers how so much of our time is spent working on work arounds and kludges to
thing that should just work.  Mozilla.org CONSTANTLY argues how they want
developers to start using Standards complaint HTML 4 and yet (in this case,
which is pretty hypocritical) we'll be forced to use spacers and the like just
to get a layout to stick if we want to make a table dynamic in *ANY WAY* (change
font sizes, animate, or hide it) ... when there may not even be room for a
spacer (and I haven't even checked if that will work to hold the spacing).

The basics of this are:  Here we have a DOM Feature, which is great to see it
implemented, essentially breaking an HTML 3.2 feature (cellpadding).  If that
isn't a regression, I don't know what is.

Think about it - the TABLE, like it or not, is the building block of every
designer/developers page out there.  This would include any positioned element
(because Netscape 4.x still won't do padding/borders right) and now if we try to
animate it, show it, hide it, or do ANYTHING to it, the padding is destroyed?
With a fix in hand?

Forgive me for my passion here, after 4 years of web building I'm just growing
tired of all the work arounds ... I hope I haven't offended anyone, I know
you're all busy, I feel like this is a stunning example of what's been wrong
with web browsers for a very long time.  And it won't be fixed for release.

It might be a good idea to change the summary to reflect the DOM-related severeness.
Agreed.  Who can do this?  I tried ... 
Summary: Enlarge/reduce text size makes cellpadding disappear → cellpadding disappears when changing style with dom, and when changing text size
Changing rtm- to rtm+ again based on recent comments.
Whiteboard: [rtm-] fix-in-hand a=buster, r=dcone → [rtm+] fix-in-hand a=buster, r=dcone
rtm-, this is still not a pull-off-the-wire issue and the train has left.
Whiteboard: [rtm+] fix-in-hand a=buster, r=dcone → [rtm-] fix-in-hand a=buster, r=dcone
a pity.  the train left too early.  and i think the rest of us missed it.
*** Bug 56725 has been marked as a duplicate of this bug. ***
Hey PDT, you suck.  Jam another buggy browser down my throat.  Fix in hand for 5
months.  PDT: the Mozilla anti-contributor.
Seconded. The fix was posted on 2000-06-05 (!!), unbelievable that it does not
make it into rtm.

I don't mind developing around bugs if all known severe ones were fixed, no
product is bugfree. But having to implement a special NS6 workaround because an
available fix HAS NOT BEEN APPLIED FOR FIVE MONTHS just sucks.

With this attitude I really hope that NS marketshare is down to some 1% or what
in a year, at least you don't have to work around totally broken features in
three browsers (IE, NS4, NS6) but only check that the page looks good on IE.

This all is really a shame. I sincerely hoped I would finally get a really good
Linux browser, and have to discover that I just get a revamped 4.5 because some
PDT guys don't get the picture (they obviously didn't change attitude since the
NS4 days).
Could this fix be checked in on the mozilla trunk now?
Jeffrey, Steven: Get your facts right. The patch was attached in October. The 
reviewing was completed exactly one month ago, not five. It was triaged by PDT 
within 30 hours, and then re-examined twice. The arguments presented here did
not convince the PDT to change their mind. To be honest, most of the arguments
were either emotional, or just plain rude. The business world doesn't work with
content-free arguments like that. The PDT specifically asked for high profile
sites that this affected, and none were given.

I've written the above in this bug so that any independent third parties that
wish to record what is going on will have both viewpoints. If you wish to argue
with my statements on particular points, please do so by e-mail or in the
newsgroups, where this belongs. Thanks.


Karnaze: Since you have r= and sr=, you can check-in now. :-)
Keywords: mozilla0.9
Whiteboard: [rtm-] fix-in-hand a=buster, r=dcone → [rtm-] fix-in-hand a=buster, r=dcone
Ian: Get your facts right. The first patch was attached in June and then three
and a half months NOTHING happened. And it's nice to know that it doesn't brake
a top100 site, but that does not change the severity of the bug. Wasn't
standards compliance the top priority (and marketing message) for NS6?

What's the message now? It doesn't brake top100 sites, so it's ok? What do you
want to tell web developers? Develop according to standards: not possible with
NS6. And the big problem is not a top100 site, they surely can spare a developer
that works around browser bugs. But if we continue to see 'Use IE to view this
page correctly' on the hundreds of thousands personal/small company web pages
then yes, we will lose the browser battle. And how do you want to convince these
people to support NS6 if it's broken?
FWIW: I've just gotten a copy of the M18 source and manually applied the
10/05/00 patch. Maybe I've made a mistake somewhere along the way, but I don't
see any change in behavior with any of the test cases. I'm running Windows 2000.
Ian, you are the one with your facts wrong.  The patch was attached on 5 June by
karnaze@netscape.com, and updated cosmetically on October 5.  As I write this,
the fix has been known for five months, four days.  At the time the fix was
created, we still had not even branched for M16.

There have been thousands and thousands of changes made to the code sine June
5.  This fix could have been among them but wasn't.  After the fix festered for
almost a half year, PDT comes along and declares that the fix is too high risk. 
This is the history of Netscape project management.  Their standard operating
procedure seems (from this developer's perspective) to be to ignore changes as
long as they must in order to say that something is too late.  We should be
clear in placing the blame here.  The fix was not too late.  The fix arrived a
half year ago.  It is PDT who is too late, and as such their inaction has
effectively cancelled out the hard work of the developer who did their job.

On to the other topic of why Netscape 6 will deserve to be booed, rejected and
boycotted.  It is because NN6 will not help web developers one single bit.  We
will not be able to sit down with a list of W3C recommedations and produce work
which renders predictably in the target browser, NN6.  Instead, we will have to
continue the now 4-year-old run-around of writing our code to the standard, and
then spending much more time trying to find out why it doesn't work in NN6. 
This is NOT an improvement.

From my other perspective as a person who has speny hundreds of unpaid hours and
a lot of emotion working on Mozilla, NN6 is a betrayal of everything I have
worked for.  NN6 is going to be by far the most used deployment of Mozilla, or
rather Mozilla as it stood six weeks ago.  As such, it will determine the future
not only of the Netscape product, but the entire Mozilla project.  If NN6 flops,
Mozilla is sunk, and along with it the hopes that many of us had for the
standardized web.

Because of my frustration with the way Netscape throttles development, I have
stopped all of my Mozilla work.  I don't even run my automated alpha-linux
nightly builds anymore.  Be aware that in at least one case, PDT behavior has
removed a contributor from the Mozilla community.
BTW, here is my list of standards that NN6 will conform to:
OS: Windows 98 → All
*** Bug 59625 has been marked as a duplicate of this bug. ***
Why isn't this being checked in on the trunk?
www.voodooextreme.com is one site that maybe this is a problem, when the 
browser window is smaller than the whole page. 
This should get checked into the trunk ASAP, since it has r= and a=.
Please stop the silly drama.  If you're quitting Mozilla because of Netscape's 
internal processes, then you were never a Mozilla contributor to begin with; 
you should have applied for a job at Netscape.
(note: bug-submitter / non-programmer totally unreliable opinion alert)
I think this bug might apply to _all_ sites that use the UBB (ultimate bulletin
board) system.  This would apply to a large, large number of sites.
www.hometheaterforum.com and www.megatokyo.com are just two that I use daily.
Can someone confirm this?
karnaze: Is there any reason why this couldn't go into the tree immediately?
I'll run the regression tests (just in case I forgot to) and check this in today 
or tomorrow. 
The patch is checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
awesome.  ...almost perfect!  :)

the patch seems to fix the problem of cellpadding when hiding, showing, and
animating the tables ... however the problem with changing font sizes still
exists - sorta - let me explain.

when manipulated from the view text size menu, the cellpadding now remains as
intended.  however when you change the text size via the DOM the padding still
disappears.

Is this a different problem (build 2000112004)?

http://www.cherny.com/mozilla/padding/padding2.html
this isn't fully resolved/fixed.  can this be updated? please see my test case
above.
I'm reopening since there still seem to be some problems
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Nom. nsbeta1. It's bad to have JS manipulation of the DOM messing up the visual
display of the document (in this case, causing cell padding to disappear), as
this means the developer must choose between *either* accessing the document
contents from JS through the DOM *or* having fidelity with intended CSS style.
Let's see if we can nail the remaining problems and close this out for nsbeta1.
Keywords: nsbeta1
WFM win98 2001021520
Yep.  I hadn't looked at this in a long time, but both testcases work for me on
Linux 2001-02-14-03.
Marking WORKSFORME as per user comments.
Status: REOPENED → RESOLVED
Closed: 24 years ago23 years ago
Resolution: --- → WORKSFORME
QA contact update
QA Contact: chrisd → amar
 Marking verified
Build Id# 2001053104
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: