Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Shouldn't modal dialog dialog.offsetParent returns <body> ? #5520

Open
sefeng211 opened this issue May 7, 2020 · 11 comments
Open

Shouldn't modal dialog dialog.offsetParent returns <body> ? #5520

sefeng211 opened this issue May 7, 2020 · 11 comments
Labels
topic: dialog The <dialog> element topic: rendering

Comments

@sefeng211
Copy link
Contributor

sefeng211 commented May 7, 2020 •
edited
Loading

In Chrome, when executing dialog.offsetParent on a modal dialog, it returns null. Shouldn't it return <body> ?

I tested this on fullscreen elements, and it would return <body> in both Chrome and Firefox.
@domenic
Copy link
Member

domenic commented May 7, 2020

The only way I could think of that this would make sense would be if all top layer elements gave offsetParent of null, which per @sefeng211's test of fullscreened elements is not true.

So our choices are to either update the offsetParent spec with a special case for modal dialogs (eww), or call this a Chrome bug. /cc @mfreed7 @chrishtr from Chrome to confirm my reasoning, and to weigh in on which of those two they would prefer.

@josepharhar
Copy link
Contributor

I'm not very familiar with offsetParent, but I debugged how null is returned with this html:

<dialog id=dialog>
  <button onclick="output.textContent = String(dialog.offsetParent)">get dialog.offsetParent</button>
  <div id=output></div>
</dialog>
<button id=opendialog onclick="dialog.showModal()">open dialog</button>
  1. The <dialog>'s LayoutObject's Parent is a #document
  2. In the first iteration of the loop walking up the parent tree, this break is chosen.
  3. #document is casted to an Element, which returns null.

I tested this on fullscreen elements, and it would return <body> in both Chrome and Firefox.

Could you show me an example of a fullscreen element so I can find out why <body> is returned in that case?

@sefeng211
Copy link
Contributor Author

Ah, okay, thanks! I see there's a bug in the test file I used, so I didn't test the fullscreen mode correctly.

I used this file. Redoing the test, I see Chrome returned null when it's in fullscreen, and firefox returned body.

Does this still look like a bug on Chrome?

@domenic
Copy link
Member

domenic commented May 11, 2020 •
edited
Loading

Interesting. Well, now it looks like an interop problem with the definition of top layer. Is anyone able to test in Safari? I'm inclined to just go with the majority behavior. I.e. if Safari matches Chrome, then update the offsetParent spec to handle top layer like 2/3 browsers do, and align Firefox to match. Whereas if Safari matches Firefox, then we should consider this a Chrome bug (for both fullscreen and modal dialogs).

/cc @emilio @zcorpan @tabatkins since this is probably going to end up in CSSWG territory soon, given that it's about https://drafts.csswg.org/cssom-view/#dom-htmlelement-offsetparent

@emilio
Copy link
Contributor

emilio commented May 11, 2020

It should return <body> per spec as long as the dialog is a descendant of the body in the flat tree (though in fairness it's not a terribly useful behavior).

However it should behave consistently with fixed-pos elements and such, and I expect stuff to be interoperable there.

@domenic
Copy link
Member

domenic commented May 11, 2020

Hmm. I think it should be consistent with fullscreened elements, and things are not interoperable there. Or are you saying that fullscreened elements should also be consistent with fixed-pos elements?

@emilio
Copy link
Contributor

emilio commented May 11, 2020

Yes, quite probably. Anything whose containing block is the viewport should behave the same I think.

@sefeng211
Copy link
Contributor Author

Safari also returns <body> https://pastebin.com/48g92Beu for fullscreen element.

@domenic
Copy link
Member

domenic commented May 12, 2020

Great! OK, then combined with @emilio's assessment, I'm going to say the spec is correct as-is, and Chrome has a bug.

@sefeng211, can you help make sure that both fullscreen and modal dialog are covered by web platform tests (fixing any broken ones you find), and add links to them here? Then I can take care of filing a Chromium bug, and we can close this spec issue.

@chrishtr
Copy link
Contributor

Changing to <body> in Chromium sounds fine to me if it achieves interop. I agree it doesn't matter much.

As for painting/stacking behavior, that does matter but that is something else. See also whatwg/fullscreen#165.

@annevk annevk added the topic: dialog The <dialog> element label May 29, 2020
@FabioGimmillaro

This comment was marked as spam.

Sign in to view
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
topic: dialog The <dialog> element topic: rendering
Milestone
No milestone
Development

No branches or pull requests
7 participants
@domenic @emilio @annevk @chrishtr @josepharhar @sefeng211 @FabioGimmillaro