Bug 86182 – [Qt] fast/frames/seamless/seamless-inherited-document-style.html fails

• Component: Tools / Tests
• Status: RESOLVED INVALID

[Csaba Osztrogonác]

New test introduced in r116694 fails on Qt:

--- /ramdisk/qt-linux-64-release/build/layout-test-results/fast/frames/seamless/seamless-inherited-document-style-expected.txt 
+++ /ramdisk/qt-linux-64-release/build/layout-test-results/fast/frames/seamless/seamless-inherited-document-style-actual.txt 
@@ -1,8 +1,8 @@
 Test that seamless iframes inherit styles from their parent iframe instead of using StyleResolver::styleForDocument defaults.
-PASS window.getComputedStyle(rootElement)['-webkit-rtl-ordering'] is "visual"
+FAIL window.getComputedStyle(rootElement)['-webkit-rtl-ordering'] should be visual. Was logical.
 FAIL window.getComputedStyle(rootElement)['-webkit-user-modify'] should be read-write. Was read-only.
-PASS window.getComputedStyle(rootElement)['-webkit-locale'] is "en_US"
-PASS window.getComputedStyle(rootElement)['writing-mode'] is "lr"
-PASS window.getComputedStyle(rootElement)['direction'] is "rtl"
-PASS window.getComputedStyle(rootElement)['font'] is "normal normal normal 18px/normal Ahem"
+FAIL window.getComputedStyle(rootElement)['-webkit-locale'] should be en_US. Was auto.
+FAIL window.getComputedStyle(rootElement)['writing-mode'] should be lr. Was lr-tb.
+FAIL window.getComputedStyle(rootElement)['direction'] should be rtl. Was ltr.
+FAIL window.getComputedStyle(rootElement)['font'] should be normal normal normal 18px/normal Ahem. Was normal normal normal 0px/normal ''.

[Csaba Osztrogonác]

I skipped it - https://trac.webkit.org/changeset/116737
Please unskip it with the proper fix.

[Eric Seidel (no email)]

It looks like you have ENABLE_IFRAME_SEAMLESS disabled. :)

[Csaba Osztrogonác]

(In reply to comment #2)
> It looks like you have ENABLE_IFRAME_SEAMLESS disabled. :)

No, ENABLE_IFRAME_SEAMLESS is true on Qt.

[Eric Seidel (no email)]

The failures are exactly those whcih would be expected if these lines of code were not executing:
https://trac.webkit.org/changeset/116694/trunk/Source/WebCore/css/StyleResolver.cpp

Perhaps the child frame's style is resolving before the parent frame's on Qt due to quirks in Widget creation.

I can offer a potential fix, but someone with a Qt build will have to try it for me. :)

[Eric Seidel (no email)]

It's possible after the latest round of seamless patches, that this may be passing now.  If Qt uses test_expectations, we could add a TEXT expectation instead of a SKIP to see.

[Eric Seidel (no email)]

If someone with a Qt build could check for me if this still fails, that would be very useful, thanks!

[Rafael Brandao]

Just ran it for Qt5+WK1, and got this diff:


 Test that seamless iframes inherit styles from their parent iframe instead of using StyleResolver::styleForDocument defaults.
-PASS window.getComputedStyle(rootElement)['-webkit-rtl-ordering'] is "visual"
+FAIL window.getComputedStyle(rootElement)['-webkit-rtl-ordering'] should be visual. Was logical.
 FAIL window.getComputedStyle(rootElement)['-webkit-user-modify'] should be read-write. Was read-only.
-PASS window.getComputedStyle(rootElement)['-webkit-locale'] is "en_US"
-PASS window.getComputedStyle(rootElement)['writing-mode'] is "lr"
-PASS window.getComputedStyle(rootElement)['direction'] is "rtl"
-PASS window.getComputedStyle(rootElement)['font'] is "normal normal normal 18px/normal Ahem"
+FAIL window.getComputedStyle(rootElement)['-webkit-locale'] should be en_US. Was auto.
+FAIL window.getComputedStyle(rootElement)['writing-mode'] should be lr. Was lr-tb.
+FAIL window.getComputedStyle(rootElement)['direction'] should be rtl. Was ltr.
+FAIL window.getComputedStyle(rootElement)['font'] should be normal normal normal 18px/normal Ahem. Was normal normal normal 0px/normal ''.

On Qt5+WK2, it is running as expected.

[Eric Seidel (no email)]

OK.  As far as we know, this is the only seamless test which fails for Qt?  I have theories...

[Rafael Brandao]

(In reply to comment #8)
> OK.  As far as we know, this is the only seamless test which fails for Qt?  I have theories...

Yep, it's the only test failing there. Please share your theories. :-)

[Eric Seidel (no email)]

This behavior is all controlled by one line in StyleResolver::styleForDocument:
http://trac.webkit.org/browser/trunk/Source/WebCore/css/StyleResolver.cpp#L1538

Presumably, Qt (due to Widget/ScrollView/FrameView) differences, is causing the style on the document to be resolved at a time when the document does not yet know if it's seamless with its parent.  And then when it's later decided that it should be, the StyleResolver is not updated.

Someone with a Qt build could set a breakpoint on that line and see it returning "false" there when we should expect it to return true. We should figure out what the stacktrace is when that is resolved.

http://trac.webkit.org/browser/trunk/Source/WebCore/html/HTMLIFrameElement.cpp#L90
is supposed to invalidate the StyleResolver (as well as the Document node's RenderStyle) when the seamless attribute is parsed/changed, but it's possible additional invalidations are necessary to cover some case Qt is tickling here.

It's possible that fixing this FIXME:
http://trac.webkit.org/browser/trunk/Source/WebCore/dom/Document.cpp#L1729
could fix this bug, as it's possible that the time at which the child document's style is resolved, the parent's style is not yet resolved.  (That FIXME would just be "if (ownerElement()) ownerElement()->document()->recalcStyle(NoChange);", except that's known to cause a crash in some tests.  If someone wanted to try replacing that fixme with those two lines and running this seamless test on Qt, that data would also be useful.)

Basically, for some reason the Document's style is not being inherited in this case, yet since all the other seamless tests are passing, document's style is being inherited at other times.  Suggesting some sort of race condition in the code, improper invalidation, or other weirdness.

It's also possible that changing from srcdoc="" to src="about:blank" (or src="some_blank_page.html" in the test could fix things, as it's possible Qt's loader is somehow handling srcdoc differently.

[Rafael Brandao]

(In reply to comment #10)
> It's possible that fixing this FIXME:
> http://trac.webkit.org/browser/trunk/Source/WebCore/dom/Document.cpp#L1729
> could fix this bug, as it's possible that the time at which the child document's style is resolved, the parent's style is not yet resolved.  (That FIXME would just be "if (ownerElement()) ownerElement()->document()->recalcStyle(NoChange);", except that's known to cause a crash in some tests.  If someone wanted to try replacing that fixme with those two lines and running this seamless test on Qt, that data would also be useful.)

That fixed the test. :) Do you have any idea of what tests are supposed to crash by using this? I've run all seamless tests, they are all running as expected.

> 
> Basically, for some reason the Document's style is not being inherited in this case, yet since all the other seamless tests are passing, document's style is being inherited at other times.  Suggesting some sort of race condition in the code, improper invalidation, or other weirdness.
> 
> It's also possible that changing from srcdoc="" to src="about:blank" (or src="some_blank_page.html" in the test could fix things, as it's possible Qt's loader is somehow handling srcdoc differently.

I've tried this first, but it didn't help.

[Eric Seidel (no email)]

(In reply to comment #11)
> (In reply to comment #10)
> > It's possible that fixing this FIXME:
> > http://trac.webkit.org/browser/trunk/Source/WebCore/dom/Document.cpp#L1729
> > could fix this bug, as it's possible that the time at which the child document's style is resolved, the parent's style is not yet resolved.  (That FIXME would just be "if (ownerElement()) ownerElement()->document()->recalcStyle(NoChange);", except that's known to cause a crash in some tests.  If someone wanted to try replacing that fixme with those two lines and running this seamless test on Qt, that data would also be useful.)
> 
> That fixed the test. :) Do you have any idea of what tests are supposed to crash by using this? I've run all seamless tests, they are all running as expected.

Oh good.  No, its' not seamless tests that crash, it's other frame-related tests.

I have a fix for that then, but I won't get to it for a couple weeks.  Thanks for the diagnosis!

[Jocelyn Turcotte]

=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.