Bug 185729 - The effect of writing-mode property remains after the property is removed (on the root element)
Summary: The effect of writing-mode property remains after the property is removed (on...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: Safari 11
Hardware: Unspecified All
: P2 Normal
Assignee: Antti Koivisto
URL:
Keywords: BrowserCompat, InRadar
: 247164 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-05-17 07:54 PDT by nanto_vi (TOYAMA Nao)
Modified: 2022-11-05 05:25 PDT (History)
20 users (show)

See Also:


Attachments
Patch (11.06 KB, patch)
2022-11-03 07:06 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
Patch (26.53 KB, patch)
2022-11-04 04:01 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
Patch (26.72 KB, patch)
2022-11-04 05:20 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
Patch (28.00 KB, patch)
2022-11-04 08:20 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
Patch (32.09 KB, patch)
2022-11-04 09:07 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
Patch (40.53 KB, patch)
2022-11-05 00:48 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
Patch (40.63 KB, patch)
2022-11-05 00:59 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
Patch (40.62 KB, patch)
2022-11-05 02:30 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
Patch (40.53 KB, patch)
2022-11-05 03:59 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description nanto_vi (TOYAMA Nao) 2018-05-17 07:54:28 PDT
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.
Comment 1 Brent Fulgham 2022-07-15 13:45:21 PDT
Safari 15.5+ continues to fail this test.
Comment 2 Radar WebKit Bug Importer 2022-07-15 13:45:55 PDT
<rdar://problem/97091990>
Comment 3 zalan 2022-07-15 20:44:13 PDT
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.
Comment 4 Karl Dubost 2022-10-27 20:36:46 PDT
*** Bug 247164 has been marked as a duplicate of this bug. ***
Comment 5 Karl Dubost 2022-10-27 20:51:39 PDT
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 = ''
Comment 6 Darin Adler 2022-10-28 05:54:07 PDT
My guess is that there is a mistake in the logic in propagateWritingModeToRenderViewIfApplicable
Comment 7 zalan 2022-10-28 06:09:12 PDT
(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.
Comment 8 zalan 2022-10-28 16:58:26 PDT
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.
Comment 9 Darin Adler 2022-10-28 17:11:53 PDT
So I guess we have to add code so if there’s no parent we fetch the initial value?
Comment 10 zalan 2022-10-28 19:55:35 PDT
(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).
Comment 12 Karl Dubost 2022-11-01 02:22:57 PDT
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
Comment 13 Antti Koivisto 2022-11-03 07:06:28 PDT
Created attachment 463387 [details]
Patch
Comment 14 Lauro Moura 2022-11-03 10:54:57 PDT
(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
Comment 15 Antti Koivisto 2022-11-04 04:01:47 PDT
Created attachment 463399 [details]
Patch
Comment 16 Antti Koivisto 2022-11-04 05:20:12 PDT
Created attachment 463400 [details]
Patch
Comment 17 Antti Koivisto 2022-11-04 08:20:47 PDT
Created attachment 463406 [details]
Patch
Comment 18 zalan 2022-11-04 08:30:43 PDT
Comment on attachment 463406 [details]
Patch

v.nice.
Comment 19 Antti Koivisto 2022-11-04 09:07:37 PDT
Created attachment 463409 [details]
Patch
Comment 20 Antti Koivisto 2022-11-05 00:48:33 PDT
Created attachment 463418 [details]
Patch
Comment 21 Antti Koivisto 2022-11-05 00:59:47 PDT
Created attachment 463419 [details]
Patch
Comment 22 Antti Koivisto 2022-11-05 02:30:33 PDT
Created attachment 463420 [details]
Patch
Comment 23 Antti Koivisto 2022-11-05 03:59:10 PDT
Created attachment 463421 [details]
Patch
Comment 24 EWS 2022-11-05 05:25:20 PDT
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].