<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/105984/">http://git.reviewboard.kde.org/r/105984/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 17th, 2012, 8:29 p.m., <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.
</pre>
<br />
<p>- Dawit</p>
<br />
<p>On August 11th, 2012, 3:58 p.m., Dawit Alemayehu wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Base Apps and David Faure.</div>
<div>By Dawit Alemayehu.</div>
<p style="color: grey;"><i>Updated Aug. 11, 2012, 3:58 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=304933">304933</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>konqueror/src/konqmainwindow.cpp <span style="color: grey">(6faba58)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105984/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>