This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.

Bug 25118 - Since this doesn't spin the event loop, the element will still be display:none when the steps try to push it to the top layer and focus it, no? (Similar for show())
Summary: Since this doesn't spin the event loop, the element will still be display:non...
Status: RESOLVED WORKSFORME
Alias: None
Product: WHATWG
Classification: Unclassified
Component: HTML (show other bugs)
Version: unspecified
Hardware: Other other
: P3 normal
Target Milestone: Unsorted
Assignee: Ian 'Hixie' Hickson
QA Contact: contributor
URL: http://www.whatwg.org/specs/web-apps/...
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-21 10:09 UTC by contributor
Modified: 2014-05-08 17:50 UTC (History)
4 users (show)

See Also:


Attachments

Description contributor 2014-03-21 10:09:18 UTC
Specification: http://www.whatwg.org/specs/web-apps/current-work/multipage/commands.html
Multipage: http://www.whatwg.org/C#dom-dialog-showmodal
Complete: http://www.whatwg.org/c#dom-dialog-showmodal
Referrer: http://www.whatwg.org/specs/web-apps/current-work/multipage/index.html

Comment:
Since this doesn't spin the event loop, the element will still be display:none
when the steps try to push it to the top layer and focus it, no? (Similar for
show())

Posted from: 90.230.218.37 by simonp@opera.com
User agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/33.0.1750.112 Safari/537.36 OPR/20.0.1387.51 (Edition Next)
Comment 1 Ian 'Hixie' Hickson 2014-05-01 19:23:11 UTC
No? The rendering doesn't update until the event loop spins, but the selectors change whether they apply immediately. Now this might mean that browsers have to do some synchronous style resolution, or lazily implement some of these requirements, or some such, but that's just an implementation detail.
Comment 3 Ian 'Hixie' Hickson 2014-05-05 21:52:46 UTC
Once the element is no longer display:none, it has CSS boxes, since CSS is defined in declarative terms. I don't understand the issue here.
Comment 4 Simon Pieters 2014-05-06 21:55:55 UTC
OK. I guess my issue is that I worry about impl bugs resulting from the impl reality not matching the spec. Should showModal() sync flush layout? Should showModal() delay running the focusing steps until the next frame? I guess the spec calls for the former but I wouldn't expect an implementor to do it that way since sync layout is bad for perf.
Comment 5 Ian 'Hixie' Hickson 2014-05-07 18:44:40 UTC
So you want this to just queue a task to do the work, basically? After adding the open="" attribute?
Comment 6 Simon Pieters 2014-05-08 08:11:10 UTC
Yeah I think that would be better (at least the focus thing). But let's ask for feedback from people implementing this...
Comment 7 Simon Pieters 2014-05-08 08:13:53 UTC
Matt, WDYT?
Comment 8 Matt Falkenhagen 2014-05-08 08:24:49 UTC
(In reply to Simon Pieters from comment #7)
> Matt, WDYT?

I'm not sure I totally understand the issue. But I can tell you the Blink implementation forces a sync layout in showModal() in order to update style to determine whether the dialog should be centered, and also to get the dialog's dimensions if it must be centered. It's always a bit gross to add a sync layout but it's not so bad if it's just once per dialog.show or dialog.showModal call.

Queuing a task may be nicer but it may also make it harder to reason about whether and where the dialog is centered, if there are style/dimension changes immediately following the showModal call.
Comment 9 Simon Pieters 2014-05-08 10:07:21 UTC
OK, thanks. Then I was wrong and the spec is fine for now.
Comment 10 Ian 'Hixie' Hickson 2014-05-08 17:50:34 UTC
If we're good with a sync layout then I'll close the bug. Thanks for the input, Matt!