--===============5799607032674029754== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable > On May 16, 2011, 9:36 p.m., Peter Penz wrote: > > The patch is fine from my point of view, but Konqueror is maintained by= David Faure -> it's up to David for a final "ship it" :-) Actually the bug here is the fact that url.fileName() is used to retrieve a= directory name. Though that does not make sense, at least not to me, it wo= rks correctly for non-root paths because fileName() ignores trailing slash= es by default. IOW, it works because by default the fileName function treat= s the last path as if it is a file, which to me, seems very hackish. Anyhow= , your fix unnecessarily creates a temporary KUrl object that will to addit= ionally parse "file:///". You could achieve the same thing without such cos= t by adding if (adjustCaption.isEmpty() && url.isLocalFile()) adjustCaption =3D QLatin1Char('/'); after the "adjustCaption =3D url.fileName()" call. - Dawit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101374/#review3354 ----------------------------------------------------------- On May 16, 2011, 9:43 p.m., Burkhard L=C3=BCck wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/101374/ > ----------------------------------------------------------- > = > (Updated May 16, 2011, 9:43 p.m.) > = > = > Review request for KDE Base Apps, David Faure and Peter Penz. > = > = > Summary > ------- > = > Konqueror in filemanager mode shows an empty tab title on browsing root (= /) folder. Dolphin displays the tab title on root (/) folder properly, so t= his patch is a copy of three lines from dolphin dolphinmainwindow.cpp. > = > = > This addresses bug 153573. > http://bugs.kde.org/show_bug.cgi?id=3D153573 > = > = > Diffs > ----- > = > konqueror/src/konqview.cpp 699c9d5 = > = > Diff: http://git.reviewboard.kde.org/r/101374/diff > = > = > Testing > ------- > = > compiled and works for me > = > = > Thanks, > = > Burkhard > = > --===============5799607032674029754== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable

This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/101374/

On May 16th, 2011, 9:36 p.m., Peter Penz wrote:

The patch= is fine from my point of view, but Konqueror is maintained by David Faure = -> it's up to David for a final "ship it" :-)

Actually th= e bug here is the fact that url.fileName() is used to retrieve a directory = name. Though that does not make sense, at least not to me, it works correct= ly for non-root paths because fileName() ignores trailing slashes by defau= lt. IOW, it works because by default the fileName function treats the last = path as if it is a file, which to me, seems very hackish. Anyhow, your fix = unnecessarily creates a temporary KUrl object that will to additionally par= se "file:///". You could achieve the same thing without such cost= by adding if (adjustCaption.isEmpty() && url.isLocalFile()) adjustCaption =3D QLatin1Char('/'); after the "adjustCaption =3D url.fileName()" call.

- Dawit

On May 16th, 2011, 9:43 p.m., Burkhard L=C3=BCck wrote:

Review request for KDE Base Apps, David Faure and Peter Penz.

By Burkhard L=C3=BCck.

Updated May 16, 2011, 9:43 p.m.

Descripti= on

Konqueror in filemanager mode shows an empty tab title on br= owsing root (/) folder. Dolphin displays the tab title on root (/) folder p= roperly, so this patch is a copy of three lines from dolphin dolphinmainwin= dow.cpp.

Testing <= /h1>

compiled and works for me

Bugs: 153573

Diffs=

konqueror/src/konqview.cpp (699c9d5)

View Diff

--===============5799607032674029754==--