Bug 429447

What should be noted is that dolphin here has an additional toolbar button by default:

Index: dolphin-19.03.60git.20201029T150039~bf11c835e/src/dolphinui.rc
===================================================================
--- dolphin-19.03.60git.20201029T150039~bf11c835e.orig/src/dolphinui.rc	2020-10-29 22:00:39.000000000 +0100
+++ dolphin-19.03.60git.20201029T150039~bf11c835e/src/dolphinui.rc	2020-10-31 09:57:20.003032608 +0100
@@ -115,6 +115,7 @@
         <text context="@title:menu">Main Toolbar</text>
         <Action name="go_back" />
         <Action name="go_forward" />
+        <Action name="go_up" />
         <Separator name="separator_1" />
         <Action name="icons" />
         <Action name="compact" />

Maybe that's what triggers this bug.

I cannot reproduce this. I am also not familiar with opensuse (I mean I know what it is but never used it; greetings to Nürnberg) or openQA. Was this bug ever confirmed by a human? Might there be an expanding spacer to the left of the URL Navigator? Can it be that the picture was taken in the 100 ms time frame Dolphin sometimes waits before adjusting the spacing? Does the problem persist after resizing Dolphin once? Is the dolphinui.rc file aside from the "up" button identical to the one from master?

(In reply to Felix Ernst from comment #2)
> I cannot reproduce this. I am also not familiar with opensuse (I mean I know
> what it is but never used it; greetings to Nürnberg) or openQA. Was this bug
> ever confirmed by a human?

Yes, I can reproduce it.

> Might there be an expanding spacer to the left of
> the URL Navigator? Can it be that the picture was taken in the 100 ms time
> frame Dolphin sometimes waits before adjusting the spacing?

No, I can interact with the window normally and it stays that way, no matter how
long I wait.

> Does the problem
> persist after resizing Dolphin once?

After changing the width or height by just 1px it snaps to the right position and
size indeed.

> Is the dolphinui.rc file aside from the
> "up" button identical to the one from master?

Yes.

> After changing the width or height by just 1px it snaps
> to the right position and size indeed.

And the next time you start Dolphin the URL Navigator is at the wrong position again?

Does clicking on a folder also make it snap to the right position?

> Does clicking on a folder also make it snap to the right position?

I mean changing the viewed folder.

(In reply to Felix Ernst from comment #4)
> > After changing the width or height by just 1px it snaps
> > to the right position and size indeed.
> 
> And the next time you start Dolphin the URL Navigator is at the wrong
> position again?

It seems like only the first start is affected, even just opening dolphin and immediately closing it again makes it work on the next start.

> Does clicking on a folder also make it snap to the right position?

No, that doesn't seem to make any difference.

I can't reproduce this either FWIW.

(In reply to Fabian Vogt from comment #6)
> It seems like only the first start is affected, even just
> opening dolphin and immediately closing it again makes
> it work on the next start.

Okay, that's good. But unfortunately this also means that debugging this touches on a lot of subjects I know very little of.

I'll write down what I know about this now so people more familiar with Dolphin startup, session restoration or Qt Application startup have an easy time resolving this bug report if they are able to reproduce it.

- The calculation of the spacing done by DolphinNavigatorsWidgetAction::adjustSpacing() is triggered whenever a viewContainer is resized or an url is changed. Because changing the folder and therefore the url didn't fix the spacing for Fabain Vogt that means that adjustSpacing() is calculating the spacing based on wrong cached geometry of the viewContainers to begin with.

- Geometry of the viewContainers is cached whenever DolphinNavigatorsWidgetAction::followViewContainerGeometry() is called. This happens only from DolphinTabPage::eventFilter() whenever a QEvent::Resize occurs for a viewContainer. This is why resizing Dolphin fixes the spacing.

- This bug occuring on the first startup therefore signifies to me that there are either resize events occuring at a time at which Qt doesn't report correct geometries yet or that no resize events are occuring at all. Whenever I start Dolphin with a single tab and a single view, a total of three calls to DolphinNavigatorsWidgetAction::followViewContainerGeometry() happen, all of which leading to the correct leading spacing of 0.

That's all I know for now.

(In reply to Nate Graham from comment #7)
> I can't reproduce this either FWIW.

I was hoping your session restoration knowledge could come in handy here. :P

I hope this doesn't happen for all new installations.

I see that the code is using mapToGlobal quite liberally. Note that mapToGlobal really uses global screen coordinates, which aren't reliable and shouldn't be used for such calculations.

I set a breakpoint at followViewContainerGeometry and it's called even before the window is visible, so mapToGlobal will return wrong values as there's no window yet. That might be the cause of this.

It probably works for resizes because the window is visible at that point and the stored window size might just trigger a resize event with similar effect.

(In reply to Fabian Vogt from comment #10)
> I see that the code is using mapToGlobal quite liberally. Note that
> mapToGlobal really uses global screen coordinates, which aren't reliable and
> shouldn't be used for such calculations.
> 
> I set a breakpoint at followViewContainerGeometry and it's called even
> before the window is visible, so mapToGlobal will return wrong values as
> there's no window yet. That might be the cause of this.
> 
> It probably works for resizes because the window is visible at that point
> and the stored window size might just trigger a resize event with similar
> effect.

This seems to be the case indeed. For the first few calls of adjustSpacing, m_globalXOfSplitter is 0 here as it's just not shown yet. By updating m_globalXOfSplitter in adjustSpacing, it's set propely on the fourth call and the spacing calculations are correct.

(In reply to Fabian Vogt from comment #11)
> (In reply to Fabian Vogt from comment #10)
> > I see that the code is using mapToGlobal quite liberally. Note that
> > mapToGlobal really uses global screen coordinates, which aren't reliable and
> > shouldn't be used for such calculations.
> > 
> > I set a breakpoint at followViewContainerGeometry and it's called even
> > before the window is visible, so mapToGlobal will return wrong values as
> > there's no window yet. That might be the cause of this.
> > 
> > It probably works for resizes because the window is visible at that point
> > and the stored window size might just trigger a resize event with similar
> > effect.
> 
> This seems to be the case indeed. For the first few calls of adjustSpacing,
> m_globalXOfSplitter is 0 here as it's just not shown yet. By updating
> m_globalXOfSplitter in adjustSpacing, it's set propely on the fourth call
> and the spacing calculations are correct.

Should I just submit ^ as workaround?

In openQA this hits in 100% of all first starts, so I expect that users will
notice this. It's not that bad as just resizing the window or restarting
dolphin fixes it, but it's literally the first impression...

Created attachment 133691 [details]
Preliminary Patch

(In reply to Fabian Vogt from comment #12)
> (In reply to Fabian Vogt from comment #11)
> > By updating
> > m_globalXOfSplitter in adjustSpacing, it's set propely on the fourth call
> > and the spacing calculations are correct.
> 
> Should I just submit ^ as workaround?

Unfortunately moving just the calculation of m_globalXOfSplitter into adjustSpacing() will introduce a different bug I ran into already: All other values are cached whenever Dolphin is resized but m_globalXOfSplitter would then change whenever adjustSpacing() is calculated, e.g. when changing the folder. Moving the window and then changing the folder would then lead to wrong spacing because m_globalXOfSplitter won't have the correct value relative to the others.

I created a patch that completely updates all cached geometry every time adjustSpacing() is called. Because that happens once on a timer 100 ms after every url change, it will happen once shortly after the window is shown. I am hoping at that point all geometry is where it should be.

Could you see if the attached patch fixes the problem?

(In reply to Felix Ernst from comment #13)
> Created attachment 133691 [details]
> Preliminary Patch
> 
> (In reply to Fabian Vogt from comment #12)
> > (In reply to Fabian Vogt from comment #11)
> > > By updating
> > > m_globalXOfSplitter in adjustSpacing, it's set propely on the fourth call
> > > and the spacing calculations are correct.
> > 
> > Should I just submit ^ as workaround?
> 
> Unfortunately moving just the calculation of m_globalXOfSplitter into
> adjustSpacing() will introduce a different bug I ran into already: All other
> values are cached whenever Dolphin is resized but m_globalXOfSplitter would
> then change whenever adjustSpacing() is calculated, e.g. when changing the
> folder. Moving the window and then changing the folder would then lead to
> wrong spacing because m_globalXOfSplitter won't have the correct value
> relative to the others.
>
> I created a patch that completely updates all cached geometry every time
> adjustSpacing() is called. Because that happens once on a timer 100 ms after
> every url change, it will happen once shortly after the window is shown. I
> am hoping at that point all geometry is where it should be.
> 
> Could you see if the attached patch fixes the problem?

Yep, that works!

Please feel free to submit that as a merge request! :)

I'll do that. I only want to clean it up a bit first.

Sounds good, thanks!

Keep in mind that final tagging for 20.12 is in 3 days: https://community.kde.org/Schedules/release_service/20.12_Release_Schedule#Thursday.2C_December_3.2C_2020:_release_service_20.12_Tagging

(In reply to Nate Graham from comment #17)
> Sounds good, thanks!
> 
> Keep in mind that final tagging for 20.12 is in 3 days:
> https://community.kde.org/Schedules/release_service/20.
> 12_Release_Schedule#Thursday.2C_December_3.2C_2020:_release_service_20.
> 12_Tagging

It seems like the deadline was missed?

Yes. I think that's okay in this case. Unfortunately I am quite busy at the moment. Trust me when I say that I would rather contribute to KDE than dealing with what I have to deal with at the moment. I'll be able to create and deal with a MR within a month I think.

A possibly relevant merge request was started @ https://invent.kde.org/system/dolphin/-/merge_requests/144

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

Git commit 0cee94ce82ccb82afd0c4e22d77e251276e7a447 by Felix Ernst.
Committed on 06/01/2021 at 01:38.
Pushed by felixernst into branch 'master'.

Fix location bar being wrongly aligned on first startup

When starting Dolphin the very first time, the spacing in front of the
location bar is wrong. This commit fixes this by completely updating
all cached geometry every time adjustSpacing() is called. Because this
happens once on a timer 100 ms after every url change, it will happen
once shortly after the window is shown. At that point all geometry is
where it should be and spacing calculation works as expected.

The ViewContainer geometry retrieval is refactored into a small nested
helper class in DolphinNavigatorsWidgetAction by the name
ViewGeometriesHelper.

Previously the logic of that class was divided between DolphinTabPage
and DolphinNavigatorsWidgetAction.
FIXED-IN: 21.04.0

M  +128  -88   src/dolphinnavigatorswidgetaction.cpp
M  +58   -20   src/dolphinnavigatorswidgetaction.h
M  +5    -29   src/dolphintabpage.cpp
M  +0    -5    src/dolphintabpage.h

https://invent.kde.org/system/dolphin/commit/0cee94ce82ccb82afd0c4e22d77e251276e7a447