1863402 - Sign in form is not displayed on tv.apple.com

Status: RESOLVED FIXED

(Core :: Web Painting, defect, P2)

Description

[Ksenia Berezina [:ksenia]]

We've received a report in https://github.com/webcompat/web-bugs/issues/129369 where a user is unable to sign in on tv.apple.com.

To reproduce,

1. Visit https://tv.apple.com/ in Nightly 121.0a1 (2023-11-04) and click on the "Sign In" button in the top-right

Expected:
"Sign In or Sign Up" form is displayed

Actual:
The form is not displayed

From mozregression:
Last good revision: 017ed2bc0c5d9eacf5f2d764f29f65c8b6edf868
First bad revision: cd7366c6b55dcaff0f6bedd5fe2cc8ea2783be30
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=017ed2bc0c5d9eacf5f2d764f29f65c8b6edf868&tochange=cd7366c6b55dcaff0f6bedd5fe2cc8ea2783be30

Comment 1

[Ksenia Berezina [:ksenia]]

Hi Emilio, wonder if you could take a look at this, please?

Comment 2

[Emilio Cobos Álvarez (:emilio)]

So this is kind of expected given how they deal with the dialog. It seems they're trying to make a fixed transparent dialog, and they are relying on a fixed-pos inside the dialog to escape. In fact on "working builds" even in chrome or what not, you can see that the dialog is a little white square behind the backdrop.

They could use something like this instead:

#ck-modal {
  position: fixed;
  inset: 0;
  border: none;
  width: auto;
  height: auto;
  background: transparent;
  max-width: none;
  max-height: none;
}

And it'd work in all browsers. Is there any chance we could contact them? Otherwise we could add an intervention or something, perhaps, but it seems unfortunate. I'll try to find some contact too.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

[Tracking Requested - why for this release]: Pretty severe bug on apple site.

Comment 5

[Ksenia Berezina [:ksenia]]

Yeah, I'll try to find a contact as well. In the meantime I can add an intervention and would need to do an uplift to beta

Comment 6

[Karl Dubost💡 :karlcow]

This is reported and tracked at Apple by rdar://118018661

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1863402 - Simplify top layer fixed list set-up and back out bug 1799036. r=dholbert,TYLin,#layout

The top layer fixed list CB and the regular fixed list CB are always the
same (the viewport frame).

We're currently sharing frame lists for these, so also share the same
AbsoluteFrameList. This makes sure that placeholders for the fixed list
are properly ordered.

Revert bug 1799036 (for now at least), since this also fixes it in a
better, less breaking way.

While at it, also insert the top layer fixed pos list after the
non-top-layer one. This is needed so that a non-top-layer abspos element
and a top layer abspos element are laid out in the right order. We don't
need to share a list for those tho, because all top-layer abspos items
are also abspos containers themselves, so we can't get a non-top-layer
sibling a top layer abspos. This fixes a couple non-fatal asserts.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Can we hold off on the intervention for now? I think comment 7 could be uplifted, or we could back out the regressing bug for now.

Apple should still consider applying something like the fix in comment 2 tho, if only to remove the annoying tiny square that the current <dialog> imposes.

Comment 9

[Ksenia Berezina [:ksenia]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #8)

Can we hold off on the intervention for now? I think comment 7 could be uplifted, or we could back out the regressing bug for now.

Apple should still consider applying something like the fix in comment 2 tho, if only to remove the annoying tiny square that the current <dialog> imposes.

Yeah, sounds good, I won't ship the intervention then. Thanks for looking into this:)

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1863402 - Simplify top layer fixed list set-up and back out bug 1799036. r=dholbert,TYLin,#layout

The top layer fixed list CB and the regular fixed list CB are always the
same (the viewport frame). We're currently using two different
AbsoluteFrameList for these, but that's wrong. Make sure to use the same
AbsoluteFrameList. This makes sure that placeholders for the fixed list
are properly ordered.

Revert bug 1799036 (for now at least), since this also fixes that issue
in a better, less breaking way.

While at it, also insert the top layer abspos list after the
non-top-layer one. This is needed so that a non-top-layer abspos element
and a top layer abspos element are laid out in the right order.

We don't need to share a list for those tho, because all top-layer
abspos items are also abspos containers themselves, so we can't get a
non-top-layer sibling a top layer abspos. This fixes a couple non-fatal
asserts.

Comment 11

[Dianna Smith [:diannaS] - on Leave, back on August 9th]

Are we still planning landing this before b9? (tomorrow is the last day to uplift before RC week)
does this just need review?

Comment 12

[Daniel Holbert [:dholbert]]

Yeah, sorry for not getting to this sooner. I'll review before going to bed tonight.

(uplift-wise: Emilio's probably in the best position to weigh in on whether this patch vs. a clean backout (per comment 8) feels safer to uplift.)

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Let's back out the regressing bug from beta. Dianna can you do that?

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/820ffb716b9d
Simplify top layer fixed list set-up and back out bug 1799036. r=dholbert

Comment 15

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/820ffb716b9d

Comment 16

[Dianna Smith [:diannaS] - on Leave, back on August 9th]

it doesn't backout cleanly from beta :/

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Attachment: Rebased backout of regressing bug.

Comment 18

[Dianna Smith [:diannaS] - on Leave, back on August 9th]

Comment on attachment 9362879 [details]
Rebased backout of regressing bug.

Approved for backout of bug 1799036 in 120.0b9

Comment 19

[Dianna Smith [:diannaS] - on Leave, back on August 9th]

fixed by backout of bug bug 1799036 in 120 #c17
fixed by backout+patch in 121 in this ticket #c15

Comment 20

[cathiechen]

There are some cases of popover still reproducing this assertion:

1. css/css-contain/content-visibility/content-visibility-with-top-layer-in-auto-subtree-removal.html
2. https://phabricator.services.mozilla.com/D175930