User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:188.8.131.52) Gecko/20060426 Firefox/184.108.40.206 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:220.127.116.11) Gecko/20060426 Firefox/18.104.22.168 In Firefox 22.214.171.124 handling of -moz-column-count, the paragraphs following a table may write over the top of the table if "height" is set on the surrounding div. See the attached test case (derived from Wikipedia's page on Poland). If the height property is removed, or table rows deleted, the page displays as expected. Reproducible: Always Steps to Reproduce: Open the attached test case. It reproduces for me irrespective of the size of the browser window. Actual Results: Paragraphs write over the top of the table. Expected Results: Layed out the table, then the P tags after it, flowing into next column.
Created attachment 222961 [details] test case 2 in this example, the second paragraph in column 2 overwrites the first paragraph. i'm presuming this has the same underlying cause, even though the problem shows up at the top of column 2 this time (compare column 1 in the first test case).
Created attachment 224011 [details] [diff] [review] Don't attempt to split a rowgroup unless we're in paginated mode
Created attachment 224013 [details] [diff] [review] better patch Get rid of the conditional exit in SplitRowGroup, it won't be taken now
NS_ASSERTION(aPresContext->IsPaginated(), "SplitRowGroup currently supports only paged media"); - if (!aPresContext->IsPaginated()) - return NS_ERROR_NOT_IMPLEMENTED; + Robert didn't you ask me just to keep this in code? Why should we assert and not exit? I put this line to get rid off all invalid callers caused by mangleme and its siblings. So before this happens, we should ask bclary to run his tests on this patch. (The predicted outcome will be a couple of new crashes due to the OrderRowGroup blues). Btw, the elimination of OrderRowGroup is currently P1 for me.
There is only one caller to SplitRowGroup and the other part of my patch ensures that that caller never calls it with !IsPaginated(). Right?
11 years ago
Comment on attachment 224013 [details] [diff] [review] better patch sr=bzbarsky, but add: NS_PRECONDITION(aPresContext->IsPaginated(), "Don't know how to split across columns"); at the beginning of SplitRowGroup?
Alright, I moved the assertion up and made it a precondition :-)
Comment on attachment 224013 [details] [diff] [review] better patch I'd like to have this in FF2 since it's a very simple fix to a nasty columns bug.
11 years ago
Apparently this fixed bug 331446 too, which is a potential security issue, so that's another reason to take this on branch.
Comment on attachment 224013 [details] [diff] [review] better patch This should go into 1.8.0 because it fixed an important crash in bug 331446.
Checked into 1.8.1 branch.
Comment on attachment 224013 [details] [diff] [review] better patch approved for 1.8.0 branch, a=dveditz for drivers
checked into 1.8.0 branch
https://bugzilla.mozilla.org/attachment.cgi?id=222961&action=view should appear without overlapping columns Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:126.96.36.199pre) Gecko/20060821 Firefox/188.8.131.52pre verified 184.108.40.206 Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1b2) Gecko/2006082203 BonEcho/2.0b2 verified 1.8.1b2
we learned from another bug (bug 362275) that the patch will not protect against splitting in print mode :-(