User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:41.0) Gecko/20100101 Firefox/41.0 Build ID: 20150917150946 Steps to reproduce: https://jsfiddle.net/moo24/Lvtr64cy/ Note: Issue only occurs when using fieldset element and if parent form has set a fixed height. Otherwise everything is working fine. Actual results: Height of fieldset element is not re-caluclated after it is set to "auto" by Javascript. Therefore input field in fieldset is not visible. Note: Everything is working fine in FireFox 40 and other browsers. Expected results: Height of fieldset element should be calculated correctly after it is set to "auto" by Javascript. http://jsfiddle.net/moo24/Lvtr64cy/2/

Regression range on nightlies: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b0b3dcfa5557&tochange=d3228c82badd Note that this is during development of Firefox 42, which means that whatever caused this got backported to Firefox 41. Looking at that checking range, the change in bug 1172239 looks pretty suspicious: it deals with vertical sizes (at least for purposes of this testcase), and was backported to Firefox 41.

So we land in nsHTMLReflowState::InitResizeFlags where we take this branch in the if/else cascade for the BResize flag: } else if (mCBReflowState && !nsLayoutUtils::IsNonWrapperBlock(frame)) { ... SetBResize(mCBReflowState->IsBResize()); mCBReflowState is the reflow state for the fixed-height <form> so it sort of make sense that after bug 1172239 it doesn't have IsBResize() true, though I would have expected it to not be true even before bug 1172239. In any case, if we tested false for !nsLayoutUtils::IsNonWrapperBlock(frame), we'd end up in the case that does: } else if (ComputedBSize() == NS_AUTOHEIGHT) { ... SetBResize(IsBResize() || NS_SUBTREE_DIRTY(frame)); which would set BResize to true, because frame has NS_FRAME_HAS_DIRTY_CHILDREN set. This part kinda looks like a regression from bug 10209 at heart: before that the test that now does IsNonWrapperBlock was !frame->IsContainingBlock(), and fieldset had IsContainingBlock() == true. I think the right fix involves changing this condition to not make fieldsets copy their parent BResize flag. I bet we have the same issue with buttons, <select>, possibly tables... What I'm not sure is what the right condition is. We clearly want to exclude anonymous blocks here. Do we want to exclude non-replaced inlines? Anything else that should be excluded?

(In reply to Boris Zbarsky [:bz] from comment #3) > This part kinda looks like a regression from bug 10209 at heart: before that > the test that now does IsNonWrapperBlock was !frame->IsContainingBlock(), > and fieldset had IsContainingBlock() == true. > > I think the right fix involves changing this condition to not make fieldsets > copy their parent BResize flag. I bet we have the same issue with buttons, > <select>, possibly tables... > > What I'm not sure is what the right condition is. We clearly want to > exclude anonymous blocks here. Do we want to exclude non-replaced inlines? > Anything else that should be excluded? I don't think it matters for inlines because they don't check the resize flag, nor should they.

(In reply to Boris Zbarsky [:bz] from comment #3) > I think the right fix involves changing this condition to not make fieldsets > copy their parent BResize flag. I bet we have the same issue with buttons, > <select>, possibly tables... I can only see this for fieldset, probably because of the curious situation where the scrollframe is a child of the fieldset frame instead of wrapping it like normal.

Bug 1209994. Take block-wrapper path only for blocks that are wrappers. r=bz

Comment on attachment 8677231 [details] MozReview Request: Bug 1209994. Fix a couple of issues that make the percentage-height-calculation.html test unreliable. r=bz https://reviewboard.mozilla.org/r/22899/#review20609 r=me, but I have low confidence that I didn't miss some edge case. :(

https://hg.mozilla.org/integration/mozilla-inbound/rev/a8adf1f9d366b936e1f25dbd3deb7a83b8cfa0cd Bug 1209994. Take block-wrapper path only for blocks that are wrappers. r=bz

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/a59b9742c81e for https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=a8adf1f9d366 05:16:35 INFO - TEST-UNEXPECTED-FAIL | /quirks-mode/percentage-height-calculation.html | The percentage height calculation quirk, <img id=test src="{png}" height=100%> - assert_equals: quirks mode expected "184px" but got "169px" 05:16:35 INFO - onload/</<@http://web-platform.test:8000/quirks-mode/percentage-height-calculation.html:92:1 05:16:35 INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1380:20 05:16:35 INFO - test@http://web-platform.test:8000/resources/testharness.js:496:9 05:16:35 INFO - onload/<@http://web-platform.test:8000/quirks-mode/percentage-height-calculation.html:82:1 05:16:35 INFO - onload@http://web-platform.test:8000/quirks-mode/percentage-height-calculation.html:81:9 05:16:35 INFO - EventHandlerNonNull*@http://web-platform.test:8000/quirks-mode/percentage-height-calculation.html:16:5 05:16:35 INFO - TEST-UNEXPECTED-PASS | /quirks-mode/percentage-height-calculation.html | The percentage height calculation quirk, <img id=test src="{png}" height=100% border=10> - expected FAIL The accidentally backed out bug 1215699 relanded as https://hg.mozilla.org/integration/mozilla-inbound/rev/b6bf653873b5

Mac only, sigh.

That test is just broken, imo. The reason it's broken is that it uses iframes with "width:20px; height: 200px;" to do its testing. Then it puts in a 1x1 image and styles it with 100% height, and maybe a 10px border. This causes overflow to the right and a horizontal scrollbar, which can then change the height of the viewport. And on Mac this likely depends on what's going on with the magic auto-hiding scrollbars or something. I think the test should be changed to use a width of at least (220 + default body padding + vertical scrollbar width) for its iframes, to avoid issues with horizontal overflow and scrollbars appearing...

Actually, there is another source of bustage in this test. The test is assuming the image loads will be sync, and nothing requires that as far as I can tell. That's why in almost-standards mode it measures 24px instead of 1px: it's measuring the size of the "loading image" placeholder, which has a quite nonzero auto height.

Comment on attachment 8677231 [details] MozReview Request: Bug 1209994. Fix a couple of issues that make the percentage-height-calculation.html test unreliable. r=bz Bug 1209994. Fix a couple of issues that make the percentage-height-calculation.html test unreliable. r=bz

Bug 1209994. Take block-wrapper path only for blocks that are wrappers. r=bz

(In reply to Boris Zbarsky [:bz] from comment #15) > Actually, there is another source of bustage in this test. The test is > assuming the image loads will be sync, and nothing requires that as far as I > can tell. That's why in almost-standards mode it measures 24px instead of > 1px: it's measuring the size of the "loading image" placeholder, which has a > quite nonzero auto height. I figured all that out after you wrote your comments but before I read them :-)

Comment on attachment 8677231 [details] MozReview Request: Bug 1209994. Fix a couple of issues that make the percentage-height-calculation.html test unreliable. r=bz r=me though technically the HTML spec only requires the preload thing to work on a per-document basis, so this test is still making assumptions that aren't supported by the spec. Fixing that will require more surgery (e.g. making it into an async test altogether), so probably OK as a separate bug report on the wpt tests on github. You're going to need to update the .ini file for this test, I assume, to remove a bunch of failure annotations. Do a try run?

https://hg.mozilla.org/integration/mozilla-inbound/rev/e7c2ff87b559a305658877b9fb903d304af105f1 Bug 1209994. Fix a couple of issues that make the percentage-height-calculation.html test unreliable. r=bz https://hg.mozilla.org/integration/mozilla-inbound/rev/beb4cf7d167970d5c774bfe903211e8d58633829 Bug 1209994. Take block-wrapper path only for blocks that are wrappers. r=bz

https://hg.mozilla.org/mozilla-central/rev/e7c2ff87b559 https://hg.mozilla.org/mozilla-central/rev/beb4cf7d1679

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/e7c2ff87b559 https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/beb4cf7d1679

Comment on attachment 8679354 [details] MozReview Request: Bug 1209994. Take block-wrapper path only for blocks that are wrappers. r=bz Approval Request Comment [Feature/regressing bug #]: 1172239 [User impact if declined]: Incorrect layout in some cases [Describe test coverage new/current, TreeHerder]: lots of layout test coverage [Risks and why]: reasonably low risk given our test coverage and small size of the patch, but nonzero [String/UUID change made/needed]: none (Need approval for both patches in this bug)

Comment on attachment 8679354 [details] MozReview Request: Bug 1209994. Take block-wrapper path only for blocks that are wrappers. r=bz Early in the cycle, plenty of tests, let's take it. Should be in 43 beta 2.

this failed to apply to beta : grafting 313475:973bf2f68f2f "Bug 1209994 - Take block-wrapper path only for blocks that are wrappers. r=bz, a=sylvestre" merging layout/generic/nsHTMLReflowState.cpp merging layout/reftests/bugs/reftest.list warning: conflicts during merge. merging layout/reftests/bugs/reftest.list incomplete! (edit conflicts, then use 'hg resolve --mark') abort: unresolved conflicts, can't continue (use hg resolve and hg graft --continue) could you take a look, thanks !

(In reply to Carsten Book [:Tomcat] from comment #25) > https://hg.mozilla.org/releases/mozilla-aurora/rev/973bf2f68f2f that got backed out because it seems it needed both parts to land (found this out after checkin), so since this was a test only changed landed both parts again as https://hg.mozilla.org/releases/mozilla-aurora/rev/cb49672a8a4d https://hg.mozilla.org/releases/mozilla-aurora/rev/cafe4b776cac

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/cb49672a8a4d https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/cafe4b776cac

Hi, seems this cause problems during uplift to beta: grafting 313545:cb49672a8a4d "Bug 1209994 - Fix a couple of issues that make the percentage-height-calculation.html test unreliable. r=bz, a=sylvestre" merging testing/web-platform/meta/quirks-mode/percentage-height-calculation.html.ini warning: conflicts during merge. merging testing/web-platform/meta/quirks-mode/percentage-height-calculation.html.ini incomplete! (edit conflicts, then use 'hg resolve --mark') abort: unresolved conflicts, can't continue (use hg resolve and hg graft --continue) could you take a look, thanks!

https://hg.mozilla.org/releases/mozilla-beta/rev/26ef30e81468 https://hg.mozilla.org/releases/mozilla-beta/rev/278b5140a696

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #30) > https://hg.mozilla.org/releases/mozilla-beta/rev/26ef30e81468 > https://hg.mozilla.org/releases/mozilla-beta/rev/278b5140a696 setting flags for roc