Closed
Bug 733614
Opened 13 years ago
Closed 12 years ago
CSS 'height' property on a multi-column block is not respected
Categories
(Core :: Layout, defect, P1)
Tracking
()
People
(Reporter: epinal99-bugzilla2, Assigned: jwir3)
References
(Depends on 1 open bug)
Details
(Keywords: dev-doc-complete, regression, Whiteboard: [qa+])
Attachments
(11 files, 3 obsolete files)
10.75 KB,
text/html
|
Details | |
1.92 KB,
text/html
|
Details | |
183.99 KB,
image/png
|
Details | |
4.67 KB,
text/html
|
Details | |
199.17 KB,
image/png
|
Details | |
16.12 KB,
text/html
|
Details | |
22.94 KB,
patch
|
Details | Diff | Splinter Review | |
40.70 KB,
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
43.77 KB,
patch
|
dbaron
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
42.89 KB,
patch
|
jwir3
:
review+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
43.79 KB,
patch
|
jwir3
:
review+
dbaron
:
review+
akeybl
:
approval-mozilla-release+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120306 Firefox/13.0a1
Build ID: 20120306031101
Steps to reproduce:
Open the testcase (joined) in FF13 and in FF10. In FF13, the property 'height' on a CSS multi-column block is not respected and FF13 is not able to build 250px-width & 600px-height columns from the left to the right.
Ref.: https://developer.mozilla.org/en/CSS3_Columns
Component: Untriaged → Layout
Product: Firefox → Core
Hardware: x86_64 → All
Attachment #603521 -
Attachment mime type: text/plain → text/html
Comment 1•13 years ago
|
||
Regression window,
Works:
http://hg.mozilla.org/mozilla-central/rev/9f29daaecbcc
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20111226 Firefox/12.0a1 ID:20111226031002
Fails:
http://hg.mozilla.org/mozilla-central/rev/6f4f2e53694b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20111227 Firefox/12.0a1 ID:20111227031015
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9f29daaecbcc&tochange=6f4f2e53694b
Suspected: Bug 695222
Blocks: 695222
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
OS: Windows 7 → All
Version: 13 Branch → 12 Branch
After reading the specs, I'm not sure if it's a real regression:
http://www.w3.org/TR/css3-multicol/#pseudo-algorithm
Pseudo-algo steps:
(15) if (column-width != auto) and (column-count = auto) then
(16) N := max(1, floor((available-width + column-gap) / (column-width + column-gap)));
(17) W := ((available-width + column-gap) / N) - column-gap;
(18) exit;
In my testcase:
N = max(1, floor(900/250)) = 3
W = 900/3 = 300
And that's this rendering we see in FF13: 3 columns with 300px as width, 600px as height between top and the horizontal scrollbar.
Comment 3•13 years ago
|
||
http://www.w3.org/TR/css3-multicol/#pagination-and-overflow-outside-multicol clearly specified as follows.
"a declaration that constrains the column height (e.g., using ‘height’ or ‘max-height’). In this case, additional column boxes are created in the inline direction"
So I think this is a regression.
Comment 4•13 years ago
|
||
Comment 5•13 years ago
|
||
Yes, I think it is too.
Assignee: nobody → sjohnson
Assignee | ||
Updated•13 years ago
|
Priority: -- → P3
Updated•13 years ago
|
QA Contact: untriaged → layout
Updated•13 years ago
|
Updated•13 years ago
|
Comment 8•13 years ago
|
||
This is now tracking for FF12 since it may cause web regressions. I'm concerned with the P3 priority attached to this bug. Please make the case for untracking for release, or prioritize/fix this bug.
Assignee | ||
Updated•13 years ago
|
Priority: P3 → P1
Assignee | ||
Comment 9•13 years ago
|
||
Modified priority to reflect that it's tracking for FF12.
Assignee | ||
Comment 10•13 years ago
|
||
I think our best bet is probably to back bug 695222 out of Ff12 until I can prepare a fix for this. How should I go about requesting approval for this?
Assignee | ||
Comment 11•13 years ago
|
||
roc, do you agree that bug 695222 should be backed out of FF12 (beta)? Otherwise, I think we'll have a strange state where FF12 supports column-fill in a way that breaks overflow for columns, but then it would be fixed again in FF13. This inconsistency should be corrected, and I'm not confident that trying to fix this, get review, get approval, etc... all within the next 2 weeks is a great idea.
[Approval Request Comment]
Regression caused by (bug #): 695222
User impact if declined: regression will cause a strange state where overflow is semi-supported in a strange way in FF12 only
Testing completed (on m-c, etc.): n/a - this is a backout patch
Risk to taking this patch (and alternatives if risky): very low, because bug 695222 was a feature originally added starting in FF12
String changes made by this patch: none
Attachment #610589 -
Flags: approval-mozilla-beta?
(In reply to Scott Johnson (:jwir3) from comment #11)
> roc, do you agree that bug 695222 should be backed out of FF12 (beta)?
Yes.
Comment 13•13 years ago
|
||
Comment on attachment 610589 [details] [diff] [review]
b695222-backout: Backout bug 695222 on mozilla-beta
[Triage Comment]
Low risk backout for a regression that may cause web incompatibilities. Approved for Beta 12.
Attachment #610589 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee | ||
Comment 14•13 years ago
|
||
Backed out of FF12:
https://hg.mozilla.org/releases/mozilla-beta/rev/fe494e9c9714
Assignee | ||
Updated•13 years ago
|
status-firefox12:
--- → fixed
Comment 15•13 years ago
|
||
I'm adding dev-doc-needed as we already documented this and have to correct it now column-fill has been (temporary) backed out.
Keywords: dev-doc-needed
Comment 16•13 years ago
|
||
So I've removed the mention from
https://developer.mozilla.org/index.php?title=en/Firefox_12_for_developers
and added it on
https://developer.mozilla.org/index.php?title=en/Firefox_13_for_developers
and updated the compatibility table (12->13) on
https://developer.mozilla.org/en/CSS/column-fill#section_5
Keywords: dev-doc-needed → dev-doc-complete
Comment 17•13 years ago
|
||
Oh yes, if column-fill is backed out of Fx 13 later, could you flip (or set if another bug) the dev-doc-complete but to dev-doc-needed so we don't miss it.
Assignee | ||
Comment 18•13 years ago
|
||
In a situation where max-width and max-height are specified, and there is overflow from columns, should this overflow ALWAYS result in additional columns? Attached is a test case with these specified, where the columns overflow, but it seems to be improperly rendered in webkit as well.
Assignee | ||
Comment 19•13 years ago
|
||
Assignee | ||
Comment 20•13 years ago
|
||
WRT comment 18, obviously our rendering is incorrect currently, but I'm curious as to what the behavior SHOULD be.
Assignee | ||
Comment 21•13 years ago
|
||
I've made a chart of what we're missing compared to what Opera and Chrome (webkit) are missing. I'm working on a patch right now to fix this. I think I can proceed by forcing it to balance in certain special cases (where the height is restricted), as it did prior to bug 695222 landing.
Assignee | ||
Comment 22•13 years ago
|
||
This seems to fix the overflow problems with bug 695222, but I'm not sure I took it in the right direction. Basically, I added a loop in Reflow() that will run a maximum of 2 times. It runs reflow as requested (with column-fill: auto or balance), but then if there is overflow (determined after FinishAndStoreOverflow() is called, and balance was requested), it loops back to the beginning, so that it can re-reflow with column-fill: auto.
The 'auto' column fill for overflow columns seems to be what webkit and opera do for overflow columns, although I can't seem to see that it's specifically noted in the spec. It doesn't seem correct to balance a bunch of overflow columns, to me, though.
At any rate, I'd appreciate an initial review to determine if this is the right approach, and if not, perhaps some guidance as to what direction to proceed in.
Thanks, Roc!
Attachment #617650 -
Flags: review?(roc)
Comment 23•13 years ago
|
||
Backed out of FF13: https://hg.mozilla.org/releases/mozilla-beta/rev/e60ca2e387a8
Comment on attachment 617650 [details] [diff] [review]
b733614
Review of attachment 617650 [details] [diff] [review]:
-----------------------------------------------------------------
::: layout/generic/nsColumnSetFrame.cpp
@@ +359,5 @@
> nscoord colHeight = GetAvailableContentHeight(aReflowState);
> if (aReflowState.ComputedHeight() != NS_INTRINSICSIZE) {
> colHeight = aReflowState.ComputedHeight();
> + } else if (aReflowState.mComputedMaxHeight != NS_INTRINSICSIZE) {
> + colHeight = aReflowState.mComputedMaxHeight;
This change looks good.
@@ +444,5 @@
> numColumns = 1;
> }
>
> colHeight = NS_MIN(mLastBalanceHeight,
> + colHeight);
This change looks good.
@@ +966,5 @@
> + // overflows, then we want to revert to column-fill: auto. Unfortunately,
> + // we can't really tell whether overflow will happen until after
> + // FinishAndStoreOverflow() is called, so we need to try to reflow with
> + // balancing first, and then if we have overflow, reflow again with balancing
> + // disabled.
This isn't quite the right approach. The overflow we care about here is not the overflow of FinishAndStoreOverflow. For example, the very first column might contain a relatively positioned element that extends outside the column set: in that case, there will be scrollable or visual overflow, but that shouldn't affect our column filling behavior.
All that matters is whether all the content can be placed in the allocated number of columns. I.e., do we hit the case
if (columnCount >= aConfig.mBalanceColCount) {
// No more columns allowed here. Stop.
If we do, then we might need to use the auto-fill fallback that your patch introduces. Otherwise we don't.
Note that if columnCount >= aConfig.mBalanceColCount and it's because we hit the 'height' or 'max-height' limit we should add overflow columns, but if it's because we hit the available-height limit we should NOT add overflow columns but keep taking the "Move any of our leftover columns to our overflow list" path, so that the extra columns will appear on the next page.
Updated•13 years ago
|
status-firefox13:
--- → fixed
Comment 25•12 years ago
|
||
Backed out of FF14: http://hg.mozilla.org/releases/mozilla-beta/rev/564eb0ab7580
Updated•12 years ago
|
status-firefox14:
--- → fixed
Comment 26•12 years ago
|
||
Are you aware that the issue is still not fixed in Firefox 14 (beta channel)? I have a web site which relies on the now broken behavior, eg.: http://www.metareads.com/articles/read/dan-maharry-ddd-southwest-session-notes-3-defensive-programming-101/ef5efdf4-eb31-48d4-a88d-605d81edd651 (on Chrome it works fine).
(In reply to Alex Keybl [:akeybl] from comment #23)
> Backed out of FF13:
> https://hg.mozilla.org/releases/mozilla-beta/rev/e60ca2e387a8
(In reply to Lukas Blakk [:lsblakk] from comment #25)
> Backed out of FF14:
> http://hg.mozilla.org/releases/mozilla-beta/rev/564eb0ab7580
These alleged backouts were both indentation changes, and look nothing like the backout that happened on Firefox 12:
https://hg.mozilla.org/releases/mozilla-beta/rev/fe494e9c9714
So it looks like we shipped this in Firefox 13. Could somebody confirm?
Comment 28•12 years ago
|
||
(In reply to David Baron [:dbaron] from comment #27)
> So it looks like we shipped this in Firefox 13. Could somebody confirm?
From the testcase and checkin, this would appear to be the case.
Scott - can you prepare backout patches for all branches, including mozilla-release and mozilla-central?
status-firefox15:
--- → affected
status-firefox16:
--- → affected
tracking-firefox15:
--- → +
tracking-firefox16:
--- → +
Assignee | ||
Comment 29•12 years ago
|
||
(In reply to Alex Keybl [:akeybl] from comment #28)
> (In reply to David Baron [:dbaron] from comment #27)
> > So it looks like we shipped this in Firefox 13. Could somebody confirm?
>
> From the testcase and checkin, this would appear to be the case.
Unfortunately, yes, this is the case.
> Scott - can you prepare backout patches for all branches, including
> mozilla-release and mozilla-central?
I'm doing so right now.
Assignee | ||
Comment 30•12 years ago
|
||
Backout patch for mozilla-central
Attachment #610589 -
Attachment is obsolete: true
Attachment #632424 -
Flags: review?(dbaron)
Assignee | ||
Comment 31•12 years ago
|
||
The others are still compiling so I can verify them. As soon as they are done, I'll post them here.
Comment on attachment 632424 [details] [diff] [review]
b695222-backout-fx16: Backout patch for central
>diff --git a/dom/interfaces/css/nsIDOMCSS2Properties.idl b/dom/interfaces/css/nsIDOMCSS2Properties.idl
>--- a/dom/interfaces/css/nsIDOMCSS2Properties.idl
>+++ b/dom/interfaces/css/nsIDOMCSS2Properties.idl
This is going to need a new UUID.
>diff --git a/layout/generic/nsColumnSetFrame.cpp b/layout/generic/nsColumnSetFrame.cpp
>--- a/layout/generic/nsColumnSetFrame.cpp
>+++ b/layout/generic/nsColumnSetFrame.cpp
>@@ -326,32 +326,16 @@ nsColumnSetFrame::ChooseColumnStrategy(c
> nscoord colHeight = GetAvailableContentHeight(aReflowState);
> if (aReflowState.ComputedHeight() != NS_INTRINSICSIZE) {
> colHeight = aReflowState.ComputedHeight();
> }
>
> nscoord colGap = GetColumnGap(this, colStyle);
> PRInt32 numColumns = colStyle->mColumnCount;
>
>- bool isBalancing = colStyle->mColumnFill == NS_STYLE_COLUMN_FILL_BALANCE;
>- if (isBalancing) {
>- const PRUint32 MAX_NESTED_COLUMN_BALANCING = 2;
>- PRUint32 cnt = 1;
>- for (const nsHTMLReflowState* rs = aReflowState.parentReflowState;
>- rs && cnt < MAX_NESTED_COLUMN_BALANCING;
>- rs = rs->parentReflowState) {
>- if (rs->mFlags.mIsColumnBalancing) {
>- ++cnt;
>- }
>- }
>- if (cnt == MAX_NESTED_COLUMN_BALANCING) {
>- numColumns = 1;
>- }
>- }
>-
You should leave this, but remove the isBalancing variable (and make it always act like isBalancing is true). Otherwise you'll be reverting bug 725376.
>@@ -635,17 +615,16 @@ nsColumnSetFrame::ReflowChildren(nsHTMLR
> if (reflowNext)
> child->AddStateBits(NS_FRAME_IS_DIRTY);
>
> nsHTMLReflowState kidReflowState(PresContext(), aReflowState, child,
> availSize, availSize.width,
> aReflowState.ComputedHeight());
> kidReflowState.mFlags.mIsTopOfPage = true;
> kidReflowState.mFlags.mTableIsSplittable = false;
>- kidReflowState.mFlags.mIsColumnBalancing = aConfig.mBalanceColCount < PR_INT32_MAX;
>
> #ifdef DEBUG_roc
> printf("*** Reflowing child #%d %p: availHeight=%d\n",
> columnCount, (void*)child,availSize.height);
> #endif
Likewise, leave this piece. (Part of the same patch.)
>-== columnfill-overflow-style.html columnfill-overflow-style-ref.html
You should also remove these test files.
And you should also remove the line added to ua.css for bug 715203.
r=dbaron with that
Attachment #632424 -
Flags: review?(dbaron) → review+
Comment 33•12 years ago
|
||
(In reply to David Baron [:dbaron] from comment #32)
> Comment on attachment 632424 [details] [diff] [review]
> b695222-backout-fx16: Backout patch for central
>
> >diff --git a/dom/interfaces/css/nsIDOMCSS2Properties.idl b/dom/interfaces/css/nsIDOMCSS2Properties.idl
> >--- a/dom/interfaces/css/nsIDOMCSS2Properties.idl
> >+++ b/dom/interfaces/css/nsIDOMCSS2Properties.idl
>
> This is going to need a new UUID.
We're pursuing a backout for mozilla-release this morning - this UUID change sounds like it could break add-on/web compatibility. Is that true?
(In reply to Alex Keybl [:akeybl] from comment #33)
> (In reply to David Baron [:dbaron] from comment #32)
> > Comment on attachment 632424 [details] [diff] [review]
> > b695222-backout-fx16: Backout patch for central
> >
> > >diff --git a/dom/interfaces/css/nsIDOMCSS2Properties.idl b/dom/interfaces/css/nsIDOMCSS2Properties.idl
> > >--- a/dom/interfaces/css/nsIDOMCSS2Properties.idl
> > >+++ b/dom/interfaces/css/nsIDOMCSS2Properties.idl
> >
> > This is going to need a new UUID.
>
> We're pursuing a backout for mozilla-release this morning - this UUID change
> sounds like it could break add-on/web compatibility. Is that true?
Nothing to do with Web compatibility, and I think breaking addons is unlikely since this interface is very unlikely to be used by C++ code (although it's possible). Its UUID changes pretty much every release, and there's an alternative way to do what it does that doesn't have that problem.
Assignee | ||
Comment 35•12 years ago
|
||
Same patch, but tailored for aurora branch.
[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 695222
User impact if declined: Functionality of column-fill will regress balancing of columns in other cases.
Testing completed (on m-c, etc.): m-c, earlier releases (fx 12)
Risk to taking this patch (and alternatives if risky): Very low risk - just backing out a patch that was previously applied.
String or UUID changes made by this patch: UUID change made to nsIDOMCSS2Properties
Attachment #632732 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 36•12 years ago
|
||
Backed out on central:
https://hg.mozilla.org/mozilla-central/rev/aeaa00b5cab5
Assignee | ||
Comment 37•12 years ago
|
||
Also added the following cset which reverts a whitespace change I made accidentally in the last patch:
https://hg.mozilla.org/mozilla-central/rev/00244ceddd42
Assignee | ||
Comment 38•12 years ago
|
||
Backout patch for release branch.
[Approval Request Comment]
(See above)
Attachment #632768 -
Flags: approval-mozilla-release?
You need to be more careful with the UUIDs; if the interface is different, you want different UUIDs, so while you're correct to use the same UUID on mozilla-central and mozilla-aurora (since the interface is currently the same on both), you need a different UUID for mozilla-release and mozilla-beta (which are the same as each other, but different from mozilla-aurora and mozilla-central).
Comment on attachment 632732 [details] [diff] [review]
b695222-backout-fx15: Backout patch for aurora
>@@ -636,18 +628,18 @@ nsColumnSetFrame::ReflowChildren(nsHTMLR
> child->AddStateBits(NS_FRAME_IS_DIRTY);
>
> nsHTMLReflowState kidReflowState(PresContext(), aReflowState, child,
> availSize, availSize.width,
> aReflowState.ComputedHeight());
> kidReflowState.mFlags.mIsTopOfPage = true;
> kidReflowState.mFlags.mTableIsSplittable = false;
> kidReflowState.mFlags.mIsColumnBalancing = aConfig.mBalanceColCount < PR_INT32_MAX;
>-
>-#ifdef DEBUG_roc
>+
>+ #ifdef DEBUG_roc
> printf("*** Reflowing child #%d %p: availHeight=%d\n",
> columnCount, (void*)child,availSize.height);
> #endif
>
> // Note if the column's next in flow is not being changed by this incremental reflow.
> // This may allow the current column to avoid trying to pull lines from the next column.
> if (child->GetNextSibling() &&
> !(GetStateBits() & NS_FRAME_IS_DIRTY) &&
Undo this change (as you did in the followup on mozilla-central), and r=dbaron
Attachment #632732 -
Flags: review+
Comment on attachment 632768 [details] [diff] [review]
b695222-backout-fx13: Backout patch for release
>-[builtinclass, scriptable, uuid(76732e62-da09-4aef-850a-78b9f6d5c8cf)]
>+[builtinclass, scriptable, uuid(35b2c7f0-b56c-11e1-afa6-0800200c9a66)]
Need a different UUID here, as I said above.
>@@ -673,17 +665,17 @@ nsColumnSetFrame::ReflowChildren(nsHTMLR
> child->AddStateBits(NS_FRAME_IS_DIRTY);
>
> nsHTMLReflowState kidReflowState(PresContext(), aReflowState, child,
> availSize, availSize.width,
> aReflowState.ComputedHeight());
> kidReflowState.mFlags.mIsTopOfPage = true;
> kidReflowState.mFlags.mTableIsSplittable = false;
> kidReflowState.mFlags.mIsColumnBalancing = aConfig.mBalanceColCount < PR_INT32_MAX;
>-
>+
> #ifdef DEBUG_roc
> printf("*** Reflowing child #%d %p: availHeight=%d\n",
> columnCount, (void*)child,availSize.height);
> #endif
>
> // Note if the column's next in flow is not being changed by this incremental reflow.
> // This may allow the current column to avoid trying to pull lines from the next column.
> if (child->GetNextSibling() &&
And drop this change.
r=dbaron with that.
I expect this patch will apply as-is to mozilla-beta as well.
Attachment #632768 -
Flags: review+
Assignee | ||
Comment 42•12 years ago
|
||
(In reply to David Baron [:dbaron] from comment #39)
> You need to be more careful with the UUIDs; if the interface is different,
> you want different UUIDs, so while you're correct to use the same UUID on
> mozilla-central and mozilla-aurora (since the interface is currently the
> same on both), you need a different UUID for mozilla-release and
> mozilla-beta (which are the same as each other, but different from
> mozilla-aurora and mozilla-central).
Oh.. the UUID needs to be different on each branch? Ok, I didn't know that, I'll change them.
Assignee | ||
Comment 43•12 years ago
|
||
[Approval Request Comment]
(See above)
Attachment #632793 -
Flags: review?(dbaron)
Attachment #632793 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 44•12 years ago
|
||
(In reply to Scott Johnson (:jwir3) from comment #42)
> (In reply to David Baron [:dbaron] from comment #39)
> > You need to be more careful with the UUIDs; if the interface is different,
> > you want different UUIDs, so while you're correct to use the same UUID on
> > mozilla-central and mozilla-aurora (since the interface is currently the
> > same on both), you need a different UUID for mozilla-release and
> > mozilla-beta (which are the same as each other, but different from
> > mozilla-aurora and mozilla-central).
>
> Oh.. the UUID needs to be different on each branch? Ok, I didn't know that,
> I'll change them.
Or, rather, what I meant was that the UUIDs need to be the same for aurora and nightly and for beta and release (but those two UUIDs should be different from each other, and what they were at before).
Assignee | ||
Comment 45•12 years ago
|
||
Forgot to remove the extra line changed.
Attachment #632793 -
Attachment is obsolete: true
Attachment #632793 -
Flags: review?(dbaron)
Attachment #632793 -
Flags: approval-mozilla-beta?
Attachment #632799 -
Flags: review?(dbaron)
Attachment #632799 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 46•12 years ago
|
||
Posting patch with dbaron's requested changes.
[Approval Request Comment]
(See above)
Attachment #632768 -
Attachment is obsolete: true
Attachment #632768 -
Flags: approval-mozilla-release?
Attachment #632801 -
Flags: review+
Attachment #632801 -
Flags: approval-mozilla-release?
Comment 47•12 years ago
|
||
Comment on attachment 632801 [details] [diff] [review]
b695222-backout-fx13: Backout patch for release
[Triage Comment]
Approved for release - low risk correctness fix that returns us to our previous behavior (no moz-column-fill).
Attachment #632801 -
Flags: approval-mozilla-release? → approval-mozilla-release+
Assignee | ||
Comment 48•12 years ago
|
||
Comment on attachment 632799 [details] [diff] [review]
b695222-backout-fx14: Backout patch for beta
Same patch as the central one.
Attachment #632799 -
Flags: review?(dbaron) → review+
Updated•12 years ago
|
Attachment #632732 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•12 years ago
|
Attachment #632799 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #632801 -
Flags: review+
Assignee | ||
Comment 49•12 years ago
|
||
Pushed to aurora:
https://hg.mozilla.org/releases/mozilla-aurora/rev/65f752c3498f
Pushed to beta:
https://hg.mozilla.org/releases/mozilla-beta/rev/235f5dbfe5aa
Pushed to release:
https://hg.mozilla.org/releases/mozilla-release/rev/fa25fc204958
Followup fixes will be done as part of re-landing bug 764567.
Assignee | ||
Updated•12 years ago
|
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 50•12 years ago
|
||
Can someone please clarify the verification for the backout? Since this is a backout, the testcase should still reproduce this bug, correct?
Assignee | ||
Comment 51•12 years ago
|
||
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #50)
> Can someone please clarify the verification for the backout? Since this is a
> backout, the testcase should still reproduce this bug, correct?
Not quite. This bug is a regression caused by bug 695222. So, by backing out bug 695222, we fixed this bug. The testcase for bug 695222 should still be unfixed, but because of clutter within that bug, I opened another one.
Comment 52•12 years ago
|
||
So, just to reiterate, the testcase attached to this bug should no longer reproduce, and the testcase in bug 695222 should still reproduce?
Assignee | ||
Comment 53•12 years ago
|
||
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #52)
> So, just to reiterate, the testcase attached to this bug should no longer
> reproduce, and the testcase in bug 695222 should still reproduce?
Basically, yes, this is correct. But, since bug 695222 was an added feature, we simply don't support the 'column-fill' or '-moz-column-fill' CSS property for the time being. (There isn't really a test case for bug 695222 is what I'm trying to say).
Assignee | ||
Comment 54•12 years ago
|
||
This had to be re-backed out on beta and release due to crashtest oranges from bug 724978:
https://hg.mozilla.org/releases/mozilla-release/rev/ada112dc8c7a
and
https://hg.mozilla.org/releases/mozilla-beta/rev/a672276a745e
and then it was re-applied:
https://hg.mozilla.org/releases/mozilla-release/rev/99a198ee453c
and
https://hg.mozilla.org/releases/mozilla-beta/rev/4a4307e626c3
Comment 55•12 years ago
|
||
Backout verified for Firefox 13.0.1 as per comment 53.
Comment 56•12 years ago
|
||
Just to be sure to understand this correctly to update the doc: we don't support (-moz-)column-fill on any Fx version yet (except on Fx 13.0.0, but this is anecdotical as removed in 13.0.1)
Am I right?
Assignee | ||
Comment 57•12 years ago
|
||
(In reply to Jean-Yves Perrier [:teoli] from comment #56)
> Just to be sure to understand this correctly to update the doc: we don't
> support (-moz-)column-fill on any Fx version yet (except on Fx 13.0.0, but
> this is anecdotical as removed in 13.0.1)
>
> Am I right?
This is correct.
Comment 58•12 years ago
|
||
So if this has now been backed out of all affected branches, can we update the status flags to 'fixed'?
Assignee | ||
Comment 59•12 years ago
|
||
(In reply to Lukas Blakk [:lsblakk] from comment #58)
> So if this has now been backed out of all affected branches, can we update
> the status flags to 'fixed'?
Yep. And done. :)
Comment 60•12 years ago
|
||
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #55)
> Backout verified for Firefox 13.0.1 as per comment 53.
Able to see the issue on Nightly 2012-03-15 with the test cases from comment 0 and comment 26.
Verified fixed on FF 14b10 on Win 7, Ubuntu 12.04 and Mac OS X 10.6.
Comment 61•12 years ago
|
||
Verified fixed on FF 15b1 on Win 7, Ubuntu 12.04 and Mac OS X 10.7.4.
Comment 62•12 years ago
|
||
Verified fixed on FF 16b5 on Win 7, Ubuntu 12.04 and Mac OS X 10.7.4.
Status: RESOLVED → VERIFIED
Depends on: 1250844
Attachment #617650 -
Flags: review?(roc)
You need to log in
before you can comment on or make changes to this bug.
Description
•