--===============4716906053363352328== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105337/#review15189 ----------------------------------------------------------- Ship it! Looks good. One might argue that statusbar visibility should also be part of view profi= les (KonqViewManager::loadItem already has support for a "ShowStatusBar" co= nfig key, but in KonqFrame::saveConfig the code was commented out for lack = of a GUI to control it). However the use case you describe is really "the user showing the statusbar= again after a website hid it", not "the user wants to get rid of all statu= sbars", so I'd say let's not save this into the profile. - David Faure On June 23, 2012, 8:51 p.m., Jonathan Marten wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/105337/ > ----------------------------------------------------------- > = > (Updated June 23, 2012, 8:51 p.m.) > = > = > Review request for KDE Base Apps and David Faure. > = > = > Description > ------- > = > The referenced bug suggested this option to cover the case where web site= s opened new windows (via JS) without user interface elements, if this is t= he case there is no way to bring back the status bar which can show importa= nt information. A patch was posted (http://lists.kde.org/?l=3Dkfm-devel&m= =3D122885401907547&w=3D2) a long time ago, but it was rejected because Konq= ueror's handling of the status bar is special (each view has its own status= bar) and the patch took no account of that. > = > Hopefully this updated patch does. The menu option only toggles the stat= us bar of the current view - I did think about making it do the status bars= of all of the views simultaneously but was not sure whether this would be = the right thing to do. Of course, for a single view in the window, the opt= ion does what is expected anyway. > = > There are GUI changes but no I18N strings (the KStandardAction is used). > = > = > This addresses bug 111162. > http://bugs.kde.org/show_bug.cgi?id=3D111162 > = > = > Diffs > ----- > = > konqueror/src/konqmainwindow.h 1666370 = > konqueror/src/konqmainwindow.cpp 0b49be5 = > konqueror/src/konqueror.rc f788484 = > = > Diff: http://git.reviewboard.kde.org/r/105337/diff/ > = > = > Testing > ------- > = > Built Konqueror with these changes, tested with file management and web b= rowsing profiles with various window splits. > = > = > Thanks, > = > Jonathan Marten > = > --===============4716906053363352328== 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/105337/

Ship it!

Looks good.

One might argue that statusbar visibility should also be part of view profi=
les (KonqViewManager::loadItem already has support for a "ShowStatusBa=
r" config key, but in KonqFrame::saveConfig the code was commented out=
 for lack of a GUI to control it).
However the use case you describe is really "the user showing the stat=
usbar again after a website hid it", not "the user wants to get r=
id of all statusbars", so I'd say let's not save this into the=
 profile.

- David


On June 23rd, 2012, 8:51 p.m., Jonathan Marten wrote:

Review request for KDE Base Apps and David Faure.
By Jonathan Marten.

Updated June 23, 2012, 8:51 p.m.

Descripti= on 

The referenced bug suggested this option to cover the case w=
here web sites opened new windows (via JS) without user interface elements,=
 if this is the case there is no way to bring back the status bar which can=
 show important information.  A patch was posted (http://lists.kde.org/?l=
=3Dkfm-devel&m=3D122885401907547&w=3D2) a long time ago, but it was=
 rejected because Konqueror's handling of the status bar is special (ea=
ch view has its own status bar) and the patch took no account of that.

Hopefully this updated patch does.  The menu option only toggles the status=
 bar of the current view - I did think about making it do the status bars o=
f all of the views simultaneously but was not sure whether this would be th=
e right thing to do.  Of course, for a single view in the window, the optio=
n does what is expected anyway.

There are GUI changes but no I18N strings (the KStandardAction is used).

Testing <= /h1> 
Built Konqueror with these changes, tested with file managem=
ent and web browsing profiles with various window splits.
Bugs: 111162

Diffs=

  • konqueror/src/konqmainwindow.h (1666370)
  • konqueror/src/konqmainwindow.cpp (0b49be5)=
  • konqueror/src/konqueror.rc (f788484)

View Diff
--===============4716906053363352328==--