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)
Tracking
()
VERIFIED
WORKSFORME
M18
People
(Reporter: hyp-x, Assigned: karnaze)
References
()
Details
(Keywords: css1, testcase, Whiteboard: [rtm-] fix-in-hand a=buster, r=dcone)
Attachments
(5 files)
120 bytes,
text/html
|
Details | |
1.58 KB,
patch
|
Details | Diff | Splinter Review | |
642 bytes,
text/html
|
Details | |
1.41 KB,
patch
|
Details | Diff | Splinter Review | |
1.92 KB,
text/html
|
Details |
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.
Reporter | ||
Comment 1•24 years ago
|
||
Assignee | ||
Comment 2•24 years ago
|
||
Assignee | ||
Updated•24 years ago
|
Whiteboard: fix-in-hand
Assignee | ||
Updated•24 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → M18
Comment 3•24 years ago
|
||
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?
Comment 4•24 years ago
|
||
Assignee | ||
Comment 5•24 years ago
|
||
Assignee | ||
Comment 7•24 years ago
|
||
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.
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.
Assignee | ||
Comment 9•24 years ago
|
||
Buster, you can't see it in the patch, but the new code is actually inside an if statement that checks the frame type.
Comment 10•24 years ago
|
||
in that case, a=buster
Whiteboard: [rtm+] fix-in-hand → [rtm+] fix-in-hand a=buster
Assignee | ||
Updated•24 years ago
|
Priority: P3 → P2
Comment 11•24 years ago
|
||
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
Comment 12•24 years ago
|
||
I tried out the patch and it worked fine r=rods
Comment 13•24 years ago
|
||
Looks good to me. r=dcone
Assignee | ||
Comment 14•24 years ago
|
||
Marking rtm+ again.
Whiteboard: [rtm need info] fix-in-hand a=buster → [rtm+] fix-in-hand a=buster
Comment 15•24 years ago
|
||
rtm-, wouldn't pull it off the wire for this.
Whiteboard: [rtm+] fix-in-hand a=buster → [rtm-] fix-in-hand a=buster
Comment 16•24 years ago
|
||
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.
Assignee | ||
Comment 17•24 years ago
|
||
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
Updated•24 years ago
|
Whiteboard: [rtm+] fix-in-hand a=buster, r=dcone → [rtm-] fix-in-hand a=buster, r=dcone
Comment 18•24 years ago
|
||
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.
Comment 19•24 years ago
|
||
>> 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.
Comment 20•24 years ago
|
||
It might be a good idea to change the summary to reflect the DOM-related severeness.
Comment 21•24 years ago
|
||
Agreed. Who can do this? I tried ...
Updated•24 years ago
|
Summary: Enlarge/reduce text size makes cellpadding disappear → cellpadding disappears when changing style with dom, and when changing text size
Assignee | ||
Comment 22•24 years ago
|
||
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
Comment 23•24 years ago
|
||
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
Comment 24•24 years ago
|
||
a pity. the train left too early. and i think the rest of us missed it.
Comment 25•24 years ago
|
||
*** Bug 56725 has been marked as a duplicate of this bug. ***
Comment 26•24 years ago
|
||
Hey PDT, you suck. Jam another buggy browser down my throat. Fix in hand for 5 months. PDT: the Mozilla anti-contributor.
Comment 27•24 years ago
|
||
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).
Comment 28•24 years ago
|
||
Comment 29•24 years ago
|
||
http://www.oreilly.com/news/flanagan_1100.html
Comment 30•24 years ago
|
||
Could this fix be checked in on the mozilla trunk now?
Comment 31•24 years ago
|
||
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
Comment 32•24 years ago
|
||
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?
Comment 33•24 years ago
|
||
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.
Comment 34•24 years ago
|
||
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.
Comment 35•24 years ago
|
||
BTW, here is my list of standards that NN6 will conform to:
OS: Windows 98 → All
Comment 36•24 years ago
|
||
*** Bug 59625 has been marked as a duplicate of this bug. ***
Comment 37•24 years ago
|
||
Why isn't this being checked in on the trunk?
Comment 38•24 years ago
|
||
www.voodooextreme.com is one site that maybe this is a problem, when the browser window is smaller than the whole page.
Comment 39•24 years ago
|
||
This should get checked into the trunk ASAP, since it has r= and a=.
Comment 40•24 years ago
|
||
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.
Comment 41•24 years ago
|
||
(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?
Comment 42•24 years ago
|
||
karnaze: Is there any reason why this couldn't go into the tree immediately?
Assignee | ||
Comment 43•24 years ago
|
||
I'll run the regression tests (just in case I forgot to) and check this in today or tomorrow.
Assignee | ||
Comment 44•24 years ago
|
||
The patch is checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Comment 45•24 years ago
|
||
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
Comment 46•24 years ago
|
||
this isn't fully resolved/fixed. can this be updated? please see my test case above.
Comment 47•24 years ago
|
||
I'm reopening since there still seem to be some problems
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 48•24 years ago
|
||
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
Comment 49•23 years ago
|
||
WFM win98 2001021520
Comment 50•23 years ago
|
||
Yep. I hadn't looked at this in a long time, but both testcases work for me on Linux 2001-02-14-03.
Comment 51•23 years ago
|
||
Marking WORKSFORME as per user comments.
Status: REOPENED → RESOLVED
Closed: 24 years ago → 23 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•