Related: cool#7951 don't invalidate when creating a new view
In writer the ViewOptions are in the ViewShell and are copied when a new
ViewShell is created from that ViewShell so the dark/light-mode and doc
color are the same in a new view as the old view.
Code looks fine - but I'm a tad confused.
a) why do we not have the correct view options pushed very early in the process of creating a new view, so we don't transition through this 'Default' state ? - I suppose that's most of my confusion.
b) The patch -seems- like we avoid this invalidate by hoping that the new view is populated with theme/etc. state from a previous view - and so we hope that this matches the new view's state that is set on it (presumably coming from the JS client), and that this then sometimes avoids this invalidate on load - but surely that's rather fragile, and a proper fix for a) would be better ?
c) I can believe that the new view should invalidate on load - my concern is/was that other views got an invalidation despite their theme and/or any other state not changing when a different new user joined: but perhaps that was fixed already by some of the other two patches you pushed Caolan ? =)
As regards a proper fix for a) - I was thinking we could push the state we want (perhaps for all components since we may not know document type by then) - as a message before the 'load' command down the websocket - which I assume creates the view after a while.
Even with Ash's re-factoring of startup to speed things up further for 24.05 we will need our own JS running inside the iframe to get the settings we need down the websocket.
Paris: was some thought put into getting this state even earlier and pushing it earlier so we could be sure the view would be created with the right state, and so avoiding switching ? =)
a\) why do we not have the correct view options pushed very
early in the process of creating a new view, so we don't transition
through this 'Default' state ? - I suppose that's most of my
confusion.
IIRC, putting aside multiple views of a document entirely for the
moment, the document gets loaded by online before the browser-side
gets to set the theme it wants with, e.g.
uno .uno:ChangeTheme {"NewTheme":{"type":"string","value":"Light"}}
so we start off in a "Default" state from the online server
perspective, I toyed with making the initial mode match the core
default of "Light" a while back with e.g.
so that default matches the reality of what is initially loaded but got
derailed on following that any further
b\) The patch \-seems\- like we avoid this invalidate by hoping
that the new view is populated with theme/etc. state from a previous
view - and so we hope that this matches the new view's state that is
set on it (presumably coming from the JS client), and that this then
sometimes avoids this invalidate on load - but surely that's rather
fragile, and a proper fix for a) would be better ?
This patch is not a complete solution for all invalidations on joining
a calc document, but identifies the reason calc has an extra
invalidation over writer. There is a difference in how calc created
a new view from an old one vs how writer created a new view from an old
one. In writer the new view has the same dark/light mode as the old
one, while in calc that wasn't the case because of the difference in
how writer's ViewOptions exist only for the view and in calc there are
ViewOptions in both view and document, and a new view in calc takes the
ViewOptions from the document, whose Light/Dark never changed, not the
old view.
So presumably/possibly has online considering the new view as the same
settings as the old view from the online perspective, but from the core
perspective they have different ViewOptions, forcing core to later
invalidate when it gets the ThemeChange notification, even though it is
possibly expected to already be in that theme from the online pov. In
any case it looks best to align the two applications in how these
online-specific settings are copied to the new view when created from
an old view so everything works the same way.
To keep things simple I've moved these basically new properties out of
the calc "ViewOptions" which can continue to behave (weirdly) as
always, and move the new props into something that belongs just to the
view and new views copy these old views extra props.
c\) I can believe that the new view should invalidate on load
- my concern is/was that other views got an invalidation despite
their theme and/or any other state not changing when a different new
user joined: but perhaps that was fixed already by some of the other
two patches you pushed Caolan ? =)
One of the other patches does address a case of invalidating on some
no-change settings modification, but there is still some calc-specific
work outstanding.
As regards a proper fix for a) - I was thinking we could push the
state we want (perhaps for all components since we may not know
document type by then) - as a message before the 'load' command down
the websocket
I'm not entirely sure where a user's light/dark preference gets stored,
do know if it is even available before the first view is created?
There are other related glitches to all of this wrt "view formatting
marks" and default inline spell-checking mode I think