> On Aug. 17, 2012, 8:29 p.m., David Faure wrote:
> > Not a strong objection, but KonqViewManager::doSetActivePart is supposed to
> > do this already, so I'm surprised it doesn't work?
> >
> > If that method isn't called in your case, then OK.
Well I finally figured out why this was happening! Believe it or not it is due
to a fix for another bug in konqFrame::activateChild(). Below is the sequence
of actions that result in the aforementioned bug report and potentially also
bug# 304865.
When KonqMainWindow's slotCreateNewWindow function is called, it calls
KonqViewManager::showTab when it is configured to put the newly created tab on
top. This happens in either slotCreateNewWindow (line #1221) itself or in the
subsequent call to openUrl (line# 596).
KonqViewManager::showTab calls KonqFrameTabs::setCurrentIndex to change the
current active tab to the new tab. KonqFrameTabs::setCurrentIndex is actually
QTabWidget::setCurrentIndex which emits the signal QTabWidget::currentChanged.
KonqFrameTabs connects the QTabWidget::currentChanged signal to its slot
KonqFrameTabs::slotCurrentChanged. KonqFrameTabs::slotCurrentChanged then ends
up calling KonqFrameBase::activateChild (which is KonqFrame::activateChild).
And in KonqFrameBase::activateChild we have the offending code @ line # 226-228:
if (m_pView->url().isEmpty() || m_pView->url() == "about:blank") {
m_pView->mainWindow()->focusLocationBar(); // #84867 usability
improvement
}
>From your commit log, the above fix seem to have been applied elsewhere but
>was moved here by you so that it won't cause another regression. Commit
>revisiion de3e94e9 says:
Re-do the usability fix #84867 in a way that doesn't introduce #208821:
focus the location bar when actively (as a user) switching to an empty tab,
but not when loading a tab which will soon get content.
Fixed for: 4.5
BUG: 208821
So in short, a fix for a regression (208821) caused by a fix for a bug (84867)
is caused yet another regression (304933). Not sure what the appropriate fix
here just yet. We actually have to find a way to fix 84867 without causing the
other two regressions or any new ones for that matter.
- Dawit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105984/#review17622
-----------------------------------------------------------
On Aug. 11, 2012, 3:58 p.m., Dawit Alemayehu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105984/
> -----------------------------------------------------------
>
> (Updated Aug. 11, 2012, 3:58 p.m.)
>
>
> Review request for KDE Base Apps and David Faure.
>
>
> Description
> -------
>
> The attached patch address the bug reported in #304933. Right now if
> Konqueror is configured to open new tabs in the foreground, i.e. the "Open
> tabs in the background" option is unchecked, then the keyboard focus is put
> on the location bar instead of the view.
>
>
> This addresses bug 304933.
> http://bugs.kde.org/show_bug.cgi?id=304933
>
>
> Diffs
> -----
>
> konqueror/src/konqmainwindow.cpp 6faba58
>
> Diff: http://git.reviewboard.kde.org/r/105984/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dawit Alemayehu
>
>
