whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.16k stars 2.68k forks source link

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

Open sefeng211 opened 4 years ago

sefeng211 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

sefeng211 commented 4 years ago

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

domenic commented 4 years ago

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 commented 4 years ago

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 https://github.com/whatwg/fullscreen/issues/165.

FabioGimmillaro commented 1 year ago

Hi. Are there any updates on this issue?