Review Request 119697: Emit urlChanged signal on directory redirection in DolphinView

[Frank Reininghaus]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119697/#review64346
-----------------------------------------------------------


Thanks for the patch! At first sight, I also thought that this is the right approach, but then I noticed that updating the main window title does not work yet. This seemed weird at first. The reason is that the main window updates its title when it receives the URL navigator's urlChanged(KUrl) signal, but this signal is blocked during a redirection in DolphinViewContainer::redirect(const KUrl& oldUrl, const KUrl& newUrl).

Moreover, the DolphinView already emits a signal redirection(oldUrl, newUrl), which signals that there has been a redirection in the view.

I see two options to fix it properly:

1. In addition to your patch, add some signal to DolphinTabWidget that is emitted in DolphinTabWidget::tabUrlChanged(const KUrl& url), and that is received in the main window and is used to update the window title (could be connected directly to DolphinMainWindow::setUrlAsCaption(KUrl) maybe?). The setUrlAsCaption call in DolphinMainWindow::changeUrl(const KUrl& url) could/should then be removed, I think.

2. Instead of your patch, listen to the view's redirection(KUrl,KUrl) signal in DolphinTabPage, emit DolphinTabPage's activeViewUrlChanged signal then, and also do the change described in 1. to ensure that the URL change propagates to the main window title.

Solution 1 might be simpler to implement, but the disadvantage is that the URL navgator first receives the "redirection" signal from the view, and then the "urlChanged" signal. It ignores the latter signal because the URL has already been modified by the "redirect" signal, but the code might become more fragile when more changes are done in the future. I'm not entirely sure what the best solution is. What do you think?

- Frank Reininghaus


On Aug. 10, 2014, 6:57 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119697/
> -----------------------------------------------------------
> 
> (Updated Aug. 10, 2014, 6:57 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Bugs: 305721
>     http://bugs.kde.org/show_bug.cgi?id=305721
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> Emit urlChanged signal on directory redirection in DolphinView, we react on this signal to change the tab titles.
> 
> 
> Diffs
> -----
> 
>   dolphin/src/views/dolphinview.cpp c1f585d 
> 
> Diff: https://git.reviewboard.kde.org/r/119697/diff/
> 
> 
> Testing
> -------
> 
> Works.
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20140812/699ecff1/attachment.htm>