[Affected versions]: 47.0a1 (2016-02-04) [Affected platforms]: Win 7 x64, Ubuntu 14.04 x64, OS X 10.10.5 [Steps to reproduce]: 1. Open https://rc4.badssl.com/ 2. Click the "Advanced" button [Expected result]: The page content shouldn't be moving when the button is clicked [Actual result]: The page content is moving when the button is clicked [Regression range]: - regressed by bug 1243310

Is this bug still valid? I don't see an Advanced button in FF48 (although it's there in Release).

RC4 host is SSL_ERROR_NO_CYPHER_OVERLAP in Fx48, but the other error screens still have this problem. [Tracking Requested - why for this release]: regression.

Bug 1220481 is changing the page layout and so it might fix this, but in any case I don't think the severity of the problem here warrants tracking.

Bug 1220481 didn't fix this.

Platform triage meeting decision: Tracked for Fx47

Review commit: https://reviewboard.mozilla.org/r/50277/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/50277/

Comment on attachment 8748435 [details] MozReview Request: Bug 1246162 - Position advanced panels absolutely below the main content in about:neterror. r=Gijs Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50277/diff/1-2/

Comment on attachment 8748435 [details] MozReview Request: Bug 1246162 - Position advanced panels absolutely below the main content in about:neterror. r=Gijs https://reviewboard.mozilla.org/r/50277/#review47185 Nice!

Hi Nihanth, once this patch lands in Nightly, could you please nominate for uplift to Beta/Aurora?

https://hg.mozilla.org/integration/fx-team/rev/335e95e6cdc528b60638b2a9802071f8143fe854 Bug 1246162 - Position advanced panels absolutely below the main content in about:neterror. r=Gijs

Comment on attachment 8748435 [details] MozReview Request: Bug 1246162 - Position advanced panels absolutely below the main content in about:neterror. r=Gijs Approval Request Comment [Feature/regressing bug #]: [User impact if declined]: UI jumps when advanced button is clicked, bad experience. [Describe test coverage new/current, TreeHerder]: none [Risks and why]: Low risk. Simple UI change, tested. [String/UUID change made/needed]: none

Comment on attachment 8748435 [details] MozReview Request: Bug 1246162 - Position advanced panels absolutely below the main content in about:neterror. r=Gijs Let's uplift to both Aurora48 and Beta47

Hello Nihanth, for test coverage in the absence of automated tests, could you please mention whether you manually verified the fix or not?

Hi Pauly, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!

This isn't applying cleanly to beta. Can we get a rebased patch if this is needed for 47?

Comment on attachment 8748435 [details] MozReview Request: Bug 1246162 - Position advanced panels absolutely below the main content in about:neterror. r=Gijs I didn't request approval for uplift to beta because bug 1220481 (on which this depends) landed in 48. I can't see a way to remove the beta approval flag. This patch needs to be backported for landing on 47, would you take a completely separate/backported patch for beta? I will work on it if so.

Er, to answer your question: yes, this patch was manually tested locally.

(In reply to Nihanth Subramanya [:nhnt11] from comment #17) > Comment on attachment 8748435 [details] > MozReview Request: Bug 1246162 - Position advanced panels absolutely below > the main content in about:neterror. r=Gijs > > I didn't request approval for uplift to beta because bug 1220481 (on which > this depends) landed in 48. I can't see a way to remove the beta approval > flag. > > This patch needs to be backported for landing on 47, would you take a > completely separate/backported patch for beta? I will work on it if so. Oops, my bad. I think we should be fixing this for Fx47. Yes, let's work on a different patch for beta if that is what is needed to fix this. We can evaluate the risk of the beta patch and then decide whether to uplift to Beta or not.

Patch for beta. This does not touch the cert error page, which doesn't suffer from page jumping but instead suffers from the main content being positioned as if the advanced panel is always visible (i.e. as if the advanced panel was set to visibility: hidden rather than display: none). See bug 1247176.

Comment on attachment 8751013 [details] [diff] [review] Patch for beta Review of attachment 8751013 [details] [diff] [review]: ----------------------------------------------------------------- Does this not need the padding fix on the container to replace the margins on the #weakCryptoAdvanced panel, then? I'm assuming you've tested this, and assuming that it really doesn't need that change, r=me.

(In reply to Paul Silaghi, QA [:pauly] from comment #0) > [Regression range]: > - regressed by bug 1243310 This fix makes bug 1243310 occur again.

(In reply to Paul Silaghi, QA [:pauly] from comment #23) > (In reply to Paul Silaghi, QA [:pauly] from comment #0) > > [Regression range]: > > - regressed by bug 1243310 > This fix makes bug 1243310 occur again. I can't reproduce this on Nightly. What specifically do you mean - can you provide steps and a screenshot? The conclusion in bug 1243310 is that the "advanced" pane *not* taking up the full width of the rest of the page, AND the existing content not shifting, is the correct behaviour. On my current OS X nightly, that is exactly what happens.

49.0a1 (2016-05-11) Win 7 x64: http://i.imgur.com/MfNDkfQ.png http://i.imgur.com/3S6WfkR.png (In reply to :Gijs Kruitbosch from comment #24) > The conclusion in bug 1243310 is that the > "advanced" pane *not* taking up the full width of the rest of the page, AND > the existing content not shifting, is the correct behaviour. I thought this was the bad behavior and was fixed by bug 1243310 - just checked a nightly 47.0a1 (2016-02-02) after bug 1243310 was fixed

(In reply to Paul Silaghi, QA [:pauly] from comment #25) > 49.0a1 (2016-05-11) Win 7 x64: > http://i.imgur.com/MfNDkfQ.png > http://i.imgur.com/3S6WfkR.png It's still not clear to me what you think is wrong in these screenshots. Please be more specific.

The width of the expanded advanced section is too small - http://i.imgur.com/ZIpmJ9L.png. It should be bigger and have a fixed width, just like before the fix of this bug landed.

(In reply to Paul Silaghi, QA [:pauly] from comment #27) > The width of the expanded advanced section is too small - > http://i.imgur.com/ZIpmJ9L.png. > It should be bigger and have a fixed width, just like before the fix of this > bug landed. I don't know that that's true, but let's ask other folks. Either way - that's a separate bug from bug 1243310 which was about the alignment, not the width, of the items...

Yeah, this is a regression, the advanced panel was supposed to take the full width of the content column. It seems position: absolute; causes this, a fix is to add width: 100% to the container. Unfortunate regression. I tested the patch with a page that has a lot of advanced panel content (so that the it occupies the full width) since that's when the jump was significant. I'll file a bug...

(In reply to Nihanth Subramanya [:nhnt11] from comment #29) > Yeah, this is a regression, the advanced panel was supposed to take the full > width of the content column. It seems position: absolute; causes this, a fix > is to add width: 100% to the container. Unfortunate regression. I tested the > patch with a page that has a lot of advanced panel content (so that the it > occupies the full width) since that's when the jump was significant. I'll > file a bug... Thanks. Once you have a fix there can you update the beta patch here and resubmit that for review so we can get this on beta ASAP?

Added the width: 100% and a bottom padding (I found a case where the bottom padding is needed, probably a good idea to throw it in).

Hello Nihanth, is the new rebased beta patch ready? I'd like to uplift it in time for 47.0b7 Thursday 5/19.

Comment on attachment 8752355 [details] [diff] [review] Patch for beta v2 Bah, forgot to request approval, sorry :( Approval Request Comment [Feature/regressing bug #]: [User impact if declined]: UI jumps when advanced button is clicked, bad experience. [Describe test coverage new/current, TreeHerder]: none [Risks and why]: Low risk. Simple UI change, tested. [String/UUID change made/needed]: none

Hi Paul, could you please verify this issue is fixed as expected on 47.0b7 (hoping the fix will land by then)? Thanks!

Comment on attachment 8752355 [details] [diff] [review] Patch for beta v2 Recent P1 regression, Beta47+

The advanced panel takes the full width of the content column. Verified fixed FX 47b7 Win 7, Ubuntu 14.04, OS X 10.10.5.

Also marking this as verified on FX 48.0a2, 49.0a1 considering the follow-up bug 1272531.