Bug 60449 – Full-screen video disappears behind black screen with scrollbar, followed by crash at jerryseinfeld.com

• Component: Media
• Status: RESOLVED FIXED

[Jer Noble]

1. Navigate to <http://jerryseinfeld.com/>
2. Click one of the video titles
3. When the video is playing, click the full screen button in the bottom right

The full-screen video is obscured by a black screen with what looks like a scrollbar track on the right. Moving the mouse sometimes hides the black screen, exposing strange-looking video control HUD, but other times leads to a Web Process crash.

[Jer Noble]

<rdar://problem/9403197>

[Jer Noble]

Created attachment 92733 [details]
Patch

[Darin Adler]

Comment on attachment 92733 [details]
Patch

Normally every non-anonymous renderer has a 1-to-1 relationship with the DOM element that is returned by node(). I’m not sure it’s OK to mark something non-anonymous if it does not have that 1-to-1 relationship with a DOM element. It may fix the behavior of this one code path, but cause problems on other code paths.

Someone more expert than me in the rendering subsystem could weigh in, or we could do a bit more research about the other call sites that check the anonymous flag.

Another possibility is to add code at the call site that coalesces anonymous children.

[Jer Noble]

(In reply to comment #3)
> (From update of attachment 92733 [details])
> Normally every non-anonymous renderer has a 1-to-1 relationship with the DOM element that is returned by node(). I’m not sure it’s OK to mark something non-anonymous if it does not have that 1-to-1 relationship with a DOM element. It may fix the behavior of this one code path, but cause problems on other code paths.
> 
> Someone more expert than me in the rendering subsystem could weigh in, or we could do a bit more research about the other call sites that check the anonymous flag.
> 
> Another possibility is to add code at the call site that coalesces anonymous children.

The other option I was considering was changing the definition of "isAnonymousBlock()". List marker renders are already excluded there (for good reason: you wouldn't want your list markers getting coalesced with the nearest non-marker anonymous block).  

I don't really understand the "1-to-1" comment. I though anonymous renderers were ones whose node() function returned the document? (From the RenderObject constructor initialization of m_isAnonymous.)

[Darin Adler]

(In reply to comment #4)
> I thought anonymous renderers were ones whose node() function returned the document? (From the RenderObject constructor initialization of m_isAnonymous.)

That’s right. Generally speaking each element has exactly one non-anonymous renderer.

[Jer Noble]

Created attachment 92737 [details]
Patch

This is the other option: excluding RenderFullScreen objects from isAnonymousBlock()

[WebKit Review Bot]

Attachment 92737 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/full..." exit_code: 1

Source/WebCore/rendering/RenderObject.h:406:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jer Noble]

(In reply to comment #5)
> (In reply to comment #4)
> > I thought anonymous renderers were ones whose node() function returned the document? (From the RenderObject constructor initialization of m_isAnonymous.)
> 
> That’s right. Generally speaking each element has exactly one non-anonymous renderer.

Oh right. "Normally every non-anonymous renderer has a 1-to-1 relationship..."  I somehow converted "non-anonymous" to "anonymous" somewhere between my eyes and brain. :)

Okay, so it appears the requirements for being an anonymous block are much more strict, and already set up to exclude certain renderers that would otherwise be counted as anonymous blocks.  And the number of call sites is much more limited.

[Eric Seidel (no email)]

Comment on attachment 92737 [details]
Patch

The rules about what is anonymous vs. not seem very confusing. We recently had a MathML crasher for similar reasons (that blocks which should have been anonymous were not).  Perhaps someone can explain the anonymous renderer rules in a public place?

[Jer Noble]

Committed r86109: <http://trac.webkit.org/changeset/86109>

[Ademar Reis]

Revision r86109 cherry-picked into qtwebkit-2.2 with commit 9b3c5d2 <http://gitorious.org/webkit/qtwebkit/commit/9b3c5d2>

[Eric Seidel (no email)]

Adding jchaffraix and esprehn, who may be able to explain the rules of anonymous renderers (here or in a Wiki).