Summary: | Full-screen video disappears behind black screen with scrollbar, followed by crash at jerryseinfeld.com | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jer Noble <jer.noble> | ||||||
Component: | Media | Assignee: | Jer Noble <jer.noble> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ademar, darin, eric, esprehn, hyatt, jchaffraix, mitz, webkit.review.bot | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
URL: | http://www.jerryseinfeld.com/ | ||||||||
Attachments: |
|
Description
Jer Noble
2011-05-08 01:04:45 PDT
Created attachment 92733 [details]
Patch
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.
(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.) (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. Created attachment 92737 [details]
Patch
This is the other option: excluding RenderFullScreen objects from isAnonymousBlock()
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.
(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. 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?
Committed r86109: <http://trac.webkit.org/changeset/86109> Revision r86109 cherry-picked into qtwebkit-2.2 with commit 9b3c5d2 <http://gitorious.org/webkit/qtwebkit/commit/9b3c5d2> Adding jchaffraix and esprehn, who may be able to explain the rules of anonymous renderers (here or in a Wiki). |