Bug 185729 – The effect of writing-mode property remains after the property is removed (on the root element)

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[nanto_vi (TOYAMA Nao)]

Visit https://jsfiddle.net/7zLr6vsn/ or

1. Set `writing-mode: vertical-rl;` to the root element.
2. Then, remove writing-mode property from the element.

Actual: The text of the document is laid out vertically.

Expected: The text of the document is laid out horizontally.

Chrome, Firefox, and Edge behave as expected.

[Brent Fulgham]

Safari 15.5+ continues to fail this test.

[Radar WebKit Bug Importer]

<rdar://problem/97091990>

[zalan]

document.documentElement.style.writingMode = ''; is indeed broken. it works as long as a non-empty, value value is set e.g. "horizontal-tb".
We probably miss re-setting the computed value to the initial value, when "" is passed in.

[Karl Dubost]

*** Bug 247164 has been marked as a duplicate of this bug. ***

[Karl Dubost]

There's a test case attached to Bug 247164 
This is happening only on the html element. 
Can also be tested with 
   document.querySelector('html').style.writingMode = ''

[Darin Adler]

My guess is that there is a mistake in the logic in propagateWritingModeToRenderViewIfApplicable

[zalan]

(In reply to Darin Adler from comment #6)
> My guess is that there is a mistake in the logic in
> propagateWritingModeToRenderViewIfApplicable
The last time I looked into this, the new style still had the old, incorrect (vertical, right-to-left) writing-mode value in styleDidChange. I think the bug is in the style system. Will take a look.

[zalan]

This bug is not specific to writing-mode. Other properties fail too (e.g. 'direction'). I am not a style expert but it looks like when the inline property is "removed" by setting the value to "", we just take the parent's property value (see makeResolutionContext()&friends). However in case of the document renderer, the parent's renderer style(m_document) has the invalid (old) value.

[Darin Adler]

So I guess we have to add code so if there’s no parent we fetch the initial value?

[zalan]

(In reply to Darin Adler from comment #9)
> So I guess we have to add code so if there’s no parent we fetch the initial
> value?
Yeah, something along those lines, unless Antti has a better idea (and he can correct me if I missed something).

[Karl Dubost]

Looking at the code (mostly for myself, but sharing in case it interests someone else)

https://searchfox.org/wubkat/rev/c4da39ca9400b440d2d14664731fe2b2ed4c45f4/Source/WebCore/style/StyleResolveForDocument.cpp#75-91

https://searchfox.org/wubkat/rev/c4da39ca9400b440d2d14664731fe2b2ed4c45f4/Source/WebCore/rendering/style/RenderStyle.cpp#197-198

https://searchfox.org/wubkat/rev/c4da39ca9400b440d2d14664731fe2b2ed4c45f4/Source/WebCore/style/MatchedDeclarationsCache.cpp#59-60

https://searchfox.org/wubkat/rev/c4da39ca9400b440d2d14664731fe2b2ed4c45f4/Source/WebCore/rendering/RenderBox.cpp#358-396

[Karl Dubost]

The comment on 
https://searchfox.org/wubkat/rev/c4da39ca9400b440d2d14664731fe2b2ed4c45f4/Source/WebCore/style/MatchedDeclarationsCache.cpp#50-51



bool MatchedDeclarationsCache::isCacheable(const Element& element, const RenderStyle& style, const RenderStyle& parentStyle)
{
    // FIXME: Writing mode and direction properties modify state when applying to document element by calling
    // Document::setWritingMode/DirectionSetOnDocumentElement. We can't skip the applying by caching.

// cut for Brevity
}


Refers to something which has been removed since.
https://github.com/WebKit/WebKit/commit/2d0180a9a3dee52363070c4cc1b8c46012774d59

[Antti Koivisto]

Created attachment 463387 [details]
Patch

[Lauro Moura]

(In reply to Antti Koivisto from comment #13)
> Created attachment 463387 [details]
> Patch

This patch crashes many layout tests in GTK (actually filling the bot with core dumps).

Here's the build: https://ews-build.webkit.org/#/builders/35/builds/30355

And here's the thread that crashed for compositing/page-cache-back-crash.html:

Thread 1 (Thread 0x7fededd0ef40 (LWP 8593)):
#0  0x00007fedf98f5bb0 in WebCore::RenderView::repaintRootContents() () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#1  0x00007fedf97a3b84 in WebCore::RenderBox::styleWillChange(WebCore::StyleDifference, WebCore::RenderStyle const&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#2  0x00007fedf97b88ff in WebCore::RenderElement::initializeStyle() () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#3  0x00007fedf9a2415e in WebCore::RenderTreeUpdater::createRenderer(WebCore::Element&, WebCore::RenderStyle&&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#4  0x00007fedf9a242f1 in WebCore::RenderTreeUpdater::updateElementRenderer(WebCore::Element&, WebCore::Style::ElementUpdate const&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#5  0x00007fedf9a26459 in WebCore::RenderTreeUpdater::updateRenderTree(WebCore::ContainerNode&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#6  0x00007fedf9a26e2b in WebCore::RenderTreeUpdater::commit(std::unique_ptr<WebCore::Style::Update const, std::default_delete<WebCore::Style::Update const> >) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#7  0x00007fedf8b9efbf in WebCore::Document::updateRenderTree(std::unique_ptr<WebCore::Style::Update const, std::default_delete<WebCore::Style::Update const> >) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#8  0x00007fedf8bbb7bc in WebCore::Document::resolveStyle(WebCore::Document::ResolveStyleType) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#9  0x00007fedf8bbed5d in WebCore::Document::didBecomeCurrentDocumentInFrame() () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#10 0x00007fedf92d082c in WebCore::Frame::setDocument(WTF::RefPtr<WebCore::Document, WTF::RawPtrTraits<WebCore::Document>, WTF::DefaultRefDerefTraits<WebCore::Document> >&&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#11 0x00007fedf91b901b in WebCore::FrameLoader::open(WebCore::CachedFrameBase&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#12 0x00007fedf8dc15a8 in WebCore::CachedPage::restore(WebCore::Page&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#13 0x00007fedf91be36e in WebCore::FrameLoader::commitProvisionalLoad() () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#14 0x00007fedf91bf4a9 in WebCore::FrameLoader::continueLoadAfterNavigationPolicy(WebCore::ResourceRequest const&, WebCore::FormState*, WebCore::NavigationPolicyDecision, WebCore::AllowNavigationToInvalidURL) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#15 0x00007fedf91c6c83 in WTF::Detail::CallableWrapper<WebCore::FrameLoader::loadWithDocumentLoader(WebCore::DocumentLoader*, WebCore::FrameLoadType, WTF::RefPtr<WebCore::FormState, WTF::RawPtrTraits<WebCore::FormState>, WTF::DefaultRefDerefTraits<WebCore::FormState> >&&, WebCore::AllowNavigationToInvalidURL, WTF::CompletionHandler<void ()>&&)::{lambda(WebCore::ResourceRequest const&, WTF::WeakPtr<WebCore::FormState, WTF::DefaultWeakPtrImpl>&&, WebCore::NavigationPolicyDecision)#2}, void, WebCore::ResourceRequest&&, WTF::WeakPtr<WebCore::FormState, WTF::DefaultWeakPtrImpl>&&, WebCore::NavigationPolicyDecision>::call(WebCore::ResourceRequest&&, WTF::WeakPtr<WebCore::FormState, WTF::DefaultWeakPtrImpl>&&, WebCore::NavigationPolicyDecision) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#16 0x00007fedf91e201a in WebCore::FrameLoader::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest&&, WebCore::ResourceResponse const&, WebCore::DocumentLoader*, WTF::RefPtr<WebCore::FormState, WTF::RawPtrTraits<WebCore::FormState>, WTF::DefaultRefDerefTraits<WebCore::FormState> >&&, WTF::CompletionHandler<void (WebCore::ResourceRequest&&, WTF::WeakPtr<WebCore::FormState, WTF::DefaultWeakPtrImpl>&&, WebCore::NavigationPolicyDecision)>&&, WebCore::PolicyDecisionMode)::{lambda(WebCore::PolicyAction, WebCore::ProcessQualified<WTF::ObjectIdentifier<WebCore::LocalPolicyCheckIdentifierType> >)#1}::operator()(WebCore::PolicyAction, WebCore::ProcessQualified<WTF::ObjectIdentifier<WebCore::LocalPolicyCheckIdentifierType> >) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#17 0x00007fedf7787584 in WebKit::WebFrame::didReceivePolicyDecision(unsigned long, WebKit::PolicyDecision&&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#18 0x00007fedf70d5ffe in void IPC::handleMessage<Messages::WebPage::DidReceivePolicyDecision, WebKit::WebPage, void (WebKit::WebPage::*)(WebCore::ProcessQualified<WTF::ObjectIdentifier<WebCore::FrameIdentifierType> >, unsigned long, WebKit::PolicyDecision&&, WTF::Vector<WebKit::SandboxExtension::Handle, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&)>(IPC::Connection&, IPC::Decoder&, WebKit::WebPage*, void (WebKit::WebPage::*)(WebCore::ProcessQualified<WTF::ObjectIdentifier<WebCore::FrameIdentifierType> >, unsigned long, WebKit::PolicyDecision&&, WTF::Vector<WebKit::SandboxExtension::Handle, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&)) [clone .isra.0] () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#19 0x00007fedf70da1d2 in WebKit::WebPage::didReceiveWebPageMessage(IPC::Connection&, IPC::Decoder&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#20 0x00007fedf72dedb8 in IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#21 0x00007fedf75cc5b6 in WebKit::WebProcess::didReceiveMessage(IPC::Connection&, IPC::Decoder&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#22 0x00007fedf72d7e05 in IPC::Connection::dispatchMessage(std::unique_ptr<IPC::Decoder, std::default_delete<IPC::Decoder> >) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#23 0x00007fedf72d8ea1 in IPC::Connection::dispatchOneIncomingMessage() () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#24 0x00007fedf48c7c3a in WTF::RunLoop::performWork() () at /app/webkit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.1.so.0
#25 0x00007fedf493f1e9 in WTF::RunLoop::RunLoop()::{lambda(void*)#1}::_FUN(void*) () at /app/webkit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.1.so.0
#26 0x00007fedf493fc6f in WTF::RunLoop::{lambda(_GSource*, int (*)(void*), void*)#1}::_FUN(_GSource*, int (*)(void*), void*) () at /app/webkit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.1.so.0
#27 0x00007fedf092cc37 in g_main_dispatch (context=0x55b3d5579f80) at ../glib/gmain.c:3419
#28 g_main_context_dispatch (context=0x55b3d5579f80) at ../glib/gmain.c:4137
#29 0x00007fedf0983028 in g_main_context_iterate.constprop.0 (context=0x55b3d5579f80, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/gmain.c:4213
#30 0x00007fedf092c2af in g_main_loop_run (loop=0x55b3d55a8400) at ../glib/gmain.c:4413
#31 0x00007fedf493fda0 in WTF::RunLoop::run() () at /app/webkit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.1.so.0
#32 0x00007fedf77ad3be in int WebKit::AuxiliaryProcessMain<WebKit::WebProcessMainGtk>(int, char**) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#33 0x00007fedf02c854a in __libc_start_call_main (main=main@entry=0x55b3d3578920 <main>, argc=argc@entry=4, argv=argv@entry=0x7ffc52a45768) at ../sysdeps/nptl/libc_start_call_main.h:58
#34 0x00007fedf02c860b in __libc_start_main_impl (main=0x55b3d3578920

[Antti Koivisto]

Created attachment 463399 [details]
Patch

[Antti Koivisto]

Created attachment 463400 [details]
Patch

[Antti Koivisto]

Created attachment 463406 [details]
Patch

[zalan]

Comment on attachment 463406 [details]
Patch

v.nice.

[Antti Koivisto]

Created attachment 463409 [details]
Patch

[Antti Koivisto]

Created attachment 463418 [details]
Patch

[Antti Koivisto]

Created attachment 463419 [details]
Patch

[Antti Koivisto]

Created attachment 463420 [details]
Patch

[Antti Koivisto]

Created attachment 463421 [details]
Patch

[EWS]

Committed 256353@main (8e911dbafcc6): <https://commits.webkit.org/256353@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 463421 [details].