Re: Review Request 120439: kio_webdav: Fix folder creation on apache + mod_dav webdav server

2014-09-30 Thread Dawit Alemayehu

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

(Updated Oct. 1, 2014, 5:15 a.m.)


Status
--

This change has been marked as submitted.


Review request for kdelibs.


Bugs: 322550
http://bugs.kde.org/show_bug.cgi?id=322550


Repository: kdelibs


Description
---

The attached single line patch fixes a problem with creation of directories on 
apache + mod_dav WebDAV server that results from us not reading the response 
the server sent back once the folder create request is fulfilled. This causes 
apache to refuse any further requests.


Diffs
-

  kioslave/http/http.cpp 7e2bca7 

Diff: https://git.reviewboard.kde.org/r/120439/diff/


Testing
---

Tested on own local apache + mod_dav server.


Thanks,

Dawit Alemayehu



Re: Review Request 113413: Improved Keyboard Layout Preview

2014-09-30 Thread shivam makkar

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

(Updated Oct. 1, 2014, 2:59 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Runtime, kde-workspace and Andriy Rysin.


Repository: kde-workspace


Description
---

Improved keyboard layout preview 
added geometry feature, multi-level keys (>4) and parsed geometry file using 
Boost c++ libraries and tool tip showing symbol names.
my GSoC-13 project !


Diffs
-

  kcontrol/keyboard/preview/kbpreviewframe.cpp 2d7a11e 

Diff: https://git.reviewboard.kde.org/r/113413/diff/


Testing
---


File Attachments


English layout default geometry
  
https://git.reviewboard.kde.org/media/uploaded/files/2013/11/05/57ce3e15-26a9-46e2-ad5d-6073e04a5489__snapshot16.jpg
ukranian layout fixed 3/4 levels
  
https://git.reviewboard.kde.org/media/uploaded/files/2013/11/15/527ada38-46b1-4b96-9d03-4a429d8275a5__snapshot19.jpg
belgian fixed 3/4 level
  
https://git.reviewboard.kde.org/media/uploaded/files/2013/11/15/1ec53ee9-f2ac-4a63-bc4f-6180160c20dd__snapshot20.jpg
microsoft natural 
  
https://git.reviewboard.kde.org/media/uploaded/files/2013/11/15/aea0136b-c4f2-412e-9de7-534b64d70af0__snapshot18.jpg


Thanks,

shivam makkar



Re: Review Request 120403: [OS X] adapting KMenu::addTitle to OS X menubar specifics

2014-09-30 Thread Thomas Lübking


> On Sept. 28, 2014, 3:13 nachm., Thomas Lübking wrote:
> > kdeui/widgets/kmenu.cpp, line 220
> > 
> >
> >  mehhh: branched exits.
> 
> René J.V. Bertin wrote:
> you realise that the 2 exits don't return the exact same kind of symbol? 
> Using 1 exit point would mean copy/casting the QWidgetAction* to a QAction* 
> or vice versa.
> 
> Thomas Lübking wrote:
> QWidgetAction is a QAction, you can just have sth. like
> 
> QAction *action;
> ...
> if () {
>...
>action = widgetAction;
> } else {
>...
> }
> ...
> return action;
> 
> though in the particular case, you can also
> 
> if () {
>action = new QWidgetAction(.);
>...
>static_cast(action)->setDefaultWidget(titleButton);
>...
> } else {
>...
> }
> 
> since that seems the only position where the QWidgetAction API is 
> actually required.
> 
> René J.V. Bertin wrote:
> `action = new QAction` in that second excerpt, undoubtedly.
> 
> You'll have noticed that I followed option 1 in the updated diff.

No. This is legit:

QAction *action = new QWidgetAction(.);

QWidgetAction is *also* a QAction.

virtual functions called on "action" will be those from QWidgetAction, 
otherwise (non virtual/QWidgetAction exclusive) you must explicitly cast to 
QWidgetAction.

Same goes eg. for
QObject *o = new QWidget();


- Thomas


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


On Sept. 30, 2014, 8:42 nachm., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120403/
> ---
> 
> (Updated Sept. 30, 2014, 8:42 nachm.)
> 
> 
> Review request for KDE Software on Mac OS X, kdelibs and Qt KDE.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> ---
> 
> This is a spin-off of RR https://git.reviewboard.kde.org/r/120355/
> 
> As described in that RR, OS X cannot render menu items that were created 
> using `KMenu::addTitle`. Without a Qt patch, that function will even provoke 
> a crash. With a patch, the items are rendered barely legibly once, and then 
> render as empty space in subsequent menu openings.
> The main object of that RR is to present and discuss a workaround at the 
> client level, emulating `KMenu::addTitle`. In this RR, I present a draft 
> adaptation of that function itself.
> 
> The main goal as I see it is to modify the function just enough so that it 
> changes its behaviour for items that will or can be rendered in the Mac's 
> global menubar, using non-Qt code. Pop-up menus that are not attached to that 
> structure are rendered through Qt and can show all the style formatting they 
> can under Linux as long as it's not tied to X11 directly.
> It's probably impossible to cater to all possible use cases, so I'd propose 
> to focus on the situations we can detect from inside `KMenu::addTitle`. That 
> is, *if* we want to ease the client's burden of obtaining a sensible menu 
> item *and* we want to maintain support for the intended/expected style in the 
> menus that can actually support them. (KDevelop's context menu its Project 
> view is a prime example.)
> The other goal (secondary for KDE/Mac for the time being) is to come up with 
> a patch proposal for Qt5's `QMenu::addSection`, because its current use of 
> texted separators makes it equivalent to `QMenu::addSeparator` on OS X. (= 
> you get a separator instead of an item showing the title text.)
> 
> I have not found a way to detect reliably that a menu is attached to the 
> global menubar. There are functions that are supposed to allow this 
> (KMenu::isTopLevelMenu, QMenu::macMenu) but they don't work as expected. So 
> what I propose here is to emulate the styled menu title when adding to a 
> KMenu that is somehow associated to a KMenu belonging to a KMainWindow that 
> has a menubar. This can probably lead to false hits, and I have already 
> learned that it doesn't catch all the intended cases either (e.g. 
> MessageList->Sorting menu under KMail's View menu is apparently not yet 
> attached when addTitle is called). So I'm following Thomas's suggestion to 
> publish the draft for feedback.
> 
> In case anyone wonders about the emulation code (when notMacMenuBar==false): 
> I'm open for suggestions but this is the only approach I've found to create 
> an entry that stands out. It's not impossible to to obtain the font OS X uses 
> for menubar menu items (Lucida Grande 14; requires 2 extra functions), but 
> changing font attributes (bold, underline, overline etc) has no effect. (The 
> bold f

Re: Review Request 120403: [OS X] adapting KMenu::addTitle to OS X menubar specifics

2014-09-30 Thread René J . V . Bertin


> On Sept. 28, 2014, 5:13 p.m., Thomas Lübking wrote:
> > kdeui/widgets/kmenu.cpp, line 220
> > 
> >
> >  mehhh: branched exits.
> 
> René J.V. Bertin wrote:
> you realise that the 2 exits don't return the exact same kind of symbol? 
> Using 1 exit point would mean copy/casting the QWidgetAction* to a QAction* 
> or vice versa.
> 
> Thomas Lübking wrote:
> QWidgetAction is a QAction, you can just have sth. like
> 
> QAction *action;
> ...
> if () {
>...
>action = widgetAction;
> } else {
>...
> }
> ...
> return action;
> 
> though in the particular case, you can also
> 
> if () {
>action = new QWidgetAction(.);
>...
>static_cast(action)->setDefaultWidget(titleButton);
>...
> } else {
>...
> }
> 
> since that seems the only position where the QWidgetAction API is 
> actually required.

`action = new QAction` in that second excerpt, undoubtedly.

You'll have noticed that I followed option 1 in the updated diff.


- René J.V.


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


On Sept. 30, 2014, 10:42 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120403/
> ---
> 
> (Updated Sept. 30, 2014, 10:42 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, kdelibs and Qt KDE.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> ---
> 
> This is a spin-off of RR https://git.reviewboard.kde.org/r/120355/
> 
> As described in that RR, OS X cannot render menu items that were created 
> using `KMenu::addTitle`. Without a Qt patch, that function will even provoke 
> a crash. With a patch, the items are rendered barely legibly once, and then 
> render as empty space in subsequent menu openings.
> The main object of that RR is to present and discuss a workaround at the 
> client level, emulating `KMenu::addTitle`. In this RR, I present a draft 
> adaptation of that function itself.
> 
> The main goal as I see it is to modify the function just enough so that it 
> changes its behaviour for items that will or can be rendered in the Mac's 
> global menubar, using non-Qt code. Pop-up menus that are not attached to that 
> structure are rendered through Qt and can show all the style formatting they 
> can under Linux as long as it's not tied to X11 directly.
> It's probably impossible to cater to all possible use cases, so I'd propose 
> to focus on the situations we can detect from inside `KMenu::addTitle`. That 
> is, *if* we want to ease the client's burden of obtaining a sensible menu 
> item *and* we want to maintain support for the intended/expected style in the 
> menus that can actually support them. (KDevelop's context menu its Project 
> view is a prime example.)
> The other goal (secondary for KDE/Mac for the time being) is to come up with 
> a patch proposal for Qt5's `QMenu::addSection`, because its current use of 
> texted separators makes it equivalent to `QMenu::addSeparator` on OS X. (= 
> you get a separator instead of an item showing the title text.)
> 
> I have not found a way to detect reliably that a menu is attached to the 
> global menubar. There are functions that are supposed to allow this 
> (KMenu::isTopLevelMenu, QMenu::macMenu) but they don't work as expected. So 
> what I propose here is to emulate the styled menu title when adding to a 
> KMenu that is somehow associated to a KMenu belonging to a KMainWindow that 
> has a menubar. This can probably lead to false hits, and I have already 
> learned that it doesn't catch all the intended cases either (e.g. 
> MessageList->Sorting menu under KMail's View menu is apparently not yet 
> attached when addTitle is called). So I'm following Thomas's suggestion to 
> publish the draft for feedback.
> 
> In case anyone wonders about the emulation code (when notMacMenuBar==false): 
> I'm open for suggestions but this is the only approach I've found to create 
> an entry that stands out. It's not impossible to to obtain the font OS X uses 
> for menubar menu items (Lucida Grande 14; requires 2 extra functions), but 
> changing font attributes (bold, underline, overline etc) has no effect. (The 
> bold font version file is available, so it might be possible to get the bold 
> font rendered by specifying it as a full font spec and not as an attribute 
> but I wouldn't know how to achieve this.)
> 
> 
> Diffs
> -
> 
>   kdeui/widgets/kmenu.cpp 7dab149 
> 
> Diff: https://git.reviewboard.kde.org/r/120403/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.

Re: Review Request 120403: [OS X] adapting KMenu::addTitle to OS X menubar specifics

2014-09-30 Thread Thomas Lübking


> On Sept. 28, 2014, 3:13 nachm., Thomas Lübking wrote:
> > kdeui/widgets/kmenu.cpp, line 220
> > 
> >
> >  mehhh: branched exits.
> 
> René J.V. Bertin wrote:
> you realise that the 2 exits don't return the exact same kind of symbol? 
> Using 1 exit point would mean copy/casting the QWidgetAction* to a QAction* 
> or vice versa.

QWidgetAction is a QAction, you can just have sth. like

QAction *action;
...
if () {
   ...
   action = widgetAction;
} else {
   ...
}
...
return action;

though in the particular case, you can also

if () {
   action = new QWidgetAction(.);
   ...
   static_cast(action)->setDefaultWidget(titleButton);
   ...
} else {
   ...
}

since that seems the only position where the QWidgetAction API is actually 
required.


- Thomas


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


On Sept. 30, 2014, 8:42 nachm., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120403/
> ---
> 
> (Updated Sept. 30, 2014, 8:42 nachm.)
> 
> 
> Review request for KDE Software on Mac OS X, kdelibs and Qt KDE.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> ---
> 
> This is a spin-off of RR https://git.reviewboard.kde.org/r/120355/
> 
> As described in that RR, OS X cannot render menu items that were created 
> using `KMenu::addTitle`. Without a Qt patch, that function will even provoke 
> a crash. With a patch, the items are rendered barely legibly once, and then 
> render as empty space in subsequent menu openings.
> The main object of that RR is to present and discuss a workaround at the 
> client level, emulating `KMenu::addTitle`. In this RR, I present a draft 
> adaptation of that function itself.
> 
> The main goal as I see it is to modify the function just enough so that it 
> changes its behaviour for items that will or can be rendered in the Mac's 
> global menubar, using non-Qt code. Pop-up menus that are not attached to that 
> structure are rendered through Qt and can show all the style formatting they 
> can under Linux as long as it's not tied to X11 directly.
> It's probably impossible to cater to all possible use cases, so I'd propose 
> to focus on the situations we can detect from inside `KMenu::addTitle`. That 
> is, *if* we want to ease the client's burden of obtaining a sensible menu 
> item *and* we want to maintain support for the intended/expected style in the 
> menus that can actually support them. (KDevelop's context menu its Project 
> view is a prime example.)
> The other goal (secondary for KDE/Mac for the time being) is to come up with 
> a patch proposal for Qt5's `QMenu::addSection`, because its current use of 
> texted separators makes it equivalent to `QMenu::addSeparator` on OS X. (= 
> you get a separator instead of an item showing the title text.)
> 
> I have not found a way to detect reliably that a menu is attached to the 
> global menubar. There are functions that are supposed to allow this 
> (KMenu::isTopLevelMenu, QMenu::macMenu) but they don't work as expected. So 
> what I propose here is to emulate the styled menu title when adding to a 
> KMenu that is somehow associated to a KMenu belonging to a KMainWindow that 
> has a menubar. This can probably lead to false hits, and I have already 
> learned that it doesn't catch all the intended cases either (e.g. 
> MessageList->Sorting menu under KMail's View menu is apparently not yet 
> attached when addTitle is called). So I'm following Thomas's suggestion to 
> publish the draft for feedback.
> 
> In case anyone wonders about the emulation code (when notMacMenuBar==false): 
> I'm open for suggestions but this is the only approach I've found to create 
> an entry that stands out. It's not impossible to to obtain the font OS X uses 
> for menubar menu items (Lucida Grande 14; requires 2 extra functions), but 
> changing font attributes (bold, underline, overline etc) has no effect. (The 
> bold font version file is available, so it might be possible to get the bold 
> font rendered by specifying it as a full font spec and not as an attribute 
> but I wouldn't know how to achieve this.)
> 
> 
> Diffs
> -
> 
>   kdeui/widgets/kmenu.cpp 7dab149 
> 
> Diff: https://git.reviewboard.kde.org/r/120403/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 with kdelibs git/kde4.14 . Currently the draft does nothing on 
> other OSes, and the actual "emulation" code (when notMacMenuBar==false) can 
> go conditional if there are no other platforms where a similar approach could 
> be desired.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>



Re: Review Request 120403: [OS X] adapting KMenu::addTitle to OS X menubar specifics

2014-09-30 Thread René J . V . Bertin

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

(Updated Sept. 30, 2014, 10:42 p.m.)


Review request for KDE Software on Mac OS X, kdelibs and Qt KDE.


Changes
---

Thomas, it seems you provided the solution:

```C++
#ifdef Q_OS_MAC
static bool isMenubarMenu(QMenu *m)
{   bool ret = false;
static int level = -1;
QList checkList;
level += 1;
if (m && m->menuAction()) {
QAction *mAct = m->menuAction();
qDebug() << "##" << level << "isMenubarMenu(" << m << m->title() << ") 
menuAction=" << mAct << mAct->text();
foreach (QWidget *w, mAct->associatedWidgets()) {
if (w == m) {
goto done;
}
qDebug() << "###" << level << "associated widget" << w << 
w->windowTitle() << "parent=" << w->parentWidget();
if (QMenuBar *mb = qobject_cast(w)) {
qDebug() << " widget is QMenuBar" << mb << 
mb->windowTitle() << "with parent" << mb->parentWidget();
ret = true;
goto done;
}
else if (QMenu *mm = qobject_cast(w)) {
if (checkList.contains(mm)) {
continue;
}
checkList.append(mm);
qDebug() << "" << level << "widget is QMenu" << mm << 
mm->title() << "with parent" << mm->parentWidget();
if (isMenubarMenu(mm)) {
ret = true;
goto done;
}
}
}
}
done:;
level -= 1;
return ret;
}
#endif
```

(still with the debugging output of course)

It correctly detects even kdepim's `MessageList::Core::Widget` message list 
sort order, aggregation and theme menus (sub-sub menus of the View menu). For 
now I've kept the "normal" title format for menus not attached to a QMenubar at 
all, we'll see how often that leads to deviant context menus.


Repository: kdelibs


Description
---

This is a spin-off of RR https://git.reviewboard.kde.org/r/120355/

As described in that RR, OS X cannot render menu items that were created using 
`KMenu::addTitle`. Without a Qt patch, that function will even provoke a crash. 
With a patch, the items are rendered barely legibly once, and then render as 
empty space in subsequent menu openings.
The main object of that RR is to present and discuss a workaround at the client 
level, emulating `KMenu::addTitle`. In this RR, I present a draft adaptation of 
that function itself.

The main goal as I see it is to modify the function just enough so that it 
changes its behaviour for items that will or can be rendered in the Mac's 
global menubar, using non-Qt code. Pop-up menus that are not attached to that 
structure are rendered through Qt and can show all the style formatting they 
can under Linux as long as it's not tied to X11 directly.
It's probably impossible to cater to all possible use cases, so I'd propose to 
focus on the situations we can detect from inside `KMenu::addTitle`. That is, 
*if* we want to ease the client's burden of obtaining a sensible menu item 
*and* we want to maintain support for the intended/expected style in the menus 
that can actually support them. (KDevelop's context menu its Project view is a 
prime example.)
The other goal (secondary for KDE/Mac for the time being) is to come up with a 
patch proposal for Qt5's `QMenu::addSection`, because its current use of texted 
separators makes it equivalent to `QMenu::addSeparator` on OS X. (= you get a 
separator instead of an item showing the title text.)

I have not found a way to detect reliably that a menu is attached to the global 
menubar. There are functions that are supposed to allow this 
(KMenu::isTopLevelMenu, QMenu::macMenu) but they don't work as expected. So 
what I propose here is to emulate the styled menu title when adding to a KMenu 
that is somehow associated to a KMenu belonging to a KMainWindow that has a 
menubar. This can probably lead to false hits, and I have already learned that 
it doesn't catch all the intended cases either (e.g. MessageList->Sorting menu 
under KMail's View menu is apparently not yet attached when addTitle is 
called). So I'm following Thomas's suggestion to publish the draft for feedback.

In case anyone wonders about the emulation code (when notMacMenuBar==false): 
I'm open for suggestions but this is the only approach I've found to create an 
entry that stands out. It's not impossible to to obtain the font OS X uses for 
menubar menu items (Lucida Grande 14; requires 2 extra functions), but changing 
font attributes (bold, underline, overline etc) has no effect. (The bold font 
version file is available, so it might be possible to get the bold font 
rendered by specifying it as a full font spec and not as an attribute but I 
wouldn't know how to achieve

Re: Review Request 120403: [OS X] adapting KMenu::addTitle to OS X menubar specifics

2014-09-30 Thread René J . V . Bertin


> On Sept. 28, 2014, 5:13 p.m., Thomas Lübking wrote:
> > kdeui/widgets/kmenu.cpp, line 220
> > 
> >
> >  mehhh: branched exits.

you realise that the 2 exits don't return the exact same kind of symbol? Using 
1 exit point would mean copy/casting the QWidgetAction* to a QAction* or vice 
versa.


- René J.V.


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


On Sept. 28, 2014, 2:46 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120403/
> ---
> 
> (Updated Sept. 28, 2014, 2:46 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, kdelibs and Qt KDE.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> ---
> 
> This is a spin-off of RR https://git.reviewboard.kde.org/r/120355/
> 
> As described in that RR, OS X cannot render menu items that were created 
> using `KMenu::addTitle`. Without a Qt patch, that function will even provoke 
> a crash. With a patch, the items are rendered barely legibly once, and then 
> render as empty space in subsequent menu openings.
> The main object of that RR is to present and discuss a workaround at the 
> client level, emulating `KMenu::addTitle`. In this RR, I present a draft 
> adaptation of that function itself.
> 
> The main goal as I see it is to modify the function just enough so that it 
> changes its behaviour for items that will or can be rendered in the Mac's 
> global menubar, using non-Qt code. Pop-up menus that are not attached to that 
> structure are rendered through Qt and can show all the style formatting they 
> can under Linux as long as it's not tied to X11 directly.
> It's probably impossible to cater to all possible use cases, so I'd propose 
> to focus on the situations we can detect from inside `KMenu::addTitle`. That 
> is, *if* we want to ease the client's burden of obtaining a sensible menu 
> item *and* we want to maintain support for the intended/expected style in the 
> menus that can actually support them. (KDevelop's context menu its Project 
> view is a prime example.)
> The other goal (secondary for KDE/Mac for the time being) is to come up with 
> a patch proposal for Qt5's `QMenu::addSection`, because its current use of 
> texted separators makes it equivalent to `QMenu::addSeparator` on OS X. (= 
> you get a separator instead of an item showing the title text.)
> 
> I have not found a way to detect reliably that a menu is attached to the 
> global menubar. There are functions that are supposed to allow this 
> (KMenu::isTopLevelMenu, QMenu::macMenu) but they don't work as expected. So 
> what I propose here is to emulate the styled menu title when adding to a 
> KMenu that is somehow associated to a KMenu belonging to a KMainWindow that 
> has a menubar. This can probably lead to false hits, and I have already 
> learned that it doesn't catch all the intended cases either (e.g. 
> MessageList->Sorting menu under KMail's View menu is apparently not yet 
> attached when addTitle is called). So I'm following Thomas's suggestion to 
> publish the draft for feedback.
> 
> In case anyone wonders about the emulation code (when notMacMenuBar==false): 
> I'm open for suggestions but this is the only approach I've found to create 
> an entry that stands out. It's not impossible to to obtain the font OS X uses 
> for menubar menu items (Lucida Grande 14; requires 2 extra functions), but 
> changing font attributes (bold, underline, overline etc) has no effect. (The 
> bold font version file is available, so it might be possible to get the bold 
> font rendered by specifying it as a full font spec and not as an attribute 
> but I wouldn't know how to achieve this.)
> 
> 
> Diffs
> -
> 
>   kdeui/widgets/kmenu.cpp 7dab149 
> 
> Diff: https://git.reviewboard.kde.org/r/120403/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 with kdelibs git/kde4.14 . Currently the draft does nothing on 
> other OSes, and the actual "emulation" code (when notMacMenuBar==false) can 
> go conditional if there are no other platforms where a similar approach could 
> be desired.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>



Review Request 120439: kio_webdav: Fix folder creation on apache + mod_dav webdav server

2014-09-30 Thread Dawit Alemayehu

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

Review request for kdelibs.


Bugs: 322550
http://bugs.kde.org/show_bug.cgi?id=322550


Repository: kdelibs


Description
---

The attached single line patch fixes a problem with creation of directories on 
apache + mod_dav WebDAV server that results from us not reading the response 
the server sent back once the folder create request is fulfilled. This causes 
apache to refuse any further requests.


Diffs
-

  kioslave/http/http.cpp 7e2bca7 

Diff: https://git.reviewboard.kde.org/r/120439/diff/


Testing
---

Tested on own local apache + mod_dav server.


Thanks,

Dawit Alemayehu



Re: Review Request 120418: kio_webdav: Added 'copyFromFile' support

2014-09-30 Thread Dawit Alemayehu

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

(Updated Sept. 30, 2014, 11:41 a.m.)


Review request for kdelibs.


Bugs: 334192
http://bugs.kde.org/show_bug.cgi?id=334192


Repository: kdelibs


Description
---

This patch adds support for 'copyFromFile' which not only improve performance 
of copying files from local filesystem to a webdav server, but also fixes the 
bug reported in bug #334192.


Diffs (updated)
-

  kioslave/http/http.h c447b69 
  kioslave/http/http.cpp 7e2bca7 
  kioslave/http/webdav.protocol c0fbd11 

Diff: https://git.reviewboard.kde.org/r/120418/diff/


Testing
---

Copy file from local directory to a webdav server.


Thanks,

Dawit Alemayehu



Re: Review Request 120418: kio_webdav: Added 'copyFromFile' support

2014-09-30 Thread Dawit Alemayehu

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

(Updated Sept. 30, 2014, 11:39 a.m.)


Review request for kdelibs.


Changes
---

Reverted most of the previous change since the error was caused by incorrect 
conversion of URL protocol and nothing else.


Bugs: 334192
http://bugs.kde.org/show_bug.cgi?id=334192


Repository: kdelibs


Description
---

This patch adds support for 'copyFromFile' which not only improve performance 
of copying files from local filesystem to a webdav server, but also fixes the 
bug reported in bug #334192.


Diffs (updated)
-

  kioslave/http/http.h c447b69 
  kioslave/http/http.cpp 7e2bca7 
  kioslave/http/webdav.protocol c0fbd11 

Diff: https://git.reviewboard.kde.org/r/120418/diff/


Testing
---

Copy file from local directory to a webdav server.


Thanks,

Dawit Alemayehu



Re: Review Request 120418: kio_webdav: Added 'copyFromFile' support

2014-09-30 Thread Dawit Alemayehu

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

(Updated Sept. 30, 2014, 11:06 a.m.)


Review request for kdelibs.


Changes
---

Fixed logic errors after extensive testing.


Bugs: 334192
http://bugs.kde.org/show_bug.cgi?id=334192


Repository: kdelibs


Description
---

This patch adds support for 'copyFromFile' which not only improve performance 
of copying files from local filesystem to a webdav server, but also fixes the 
bug reported in bug #334192.


Diffs (updated)
-

  kioslave/http/webdav.protocol c0fbd11 
  kioslave/http/http.cpp 7e2bca7 
  kioslave/http/http.h c447b69 

Diff: https://git.reviewboard.kde.org/r/120418/diff/


Testing
---

Copy file from local directory to a webdav server.


Thanks,

Dawit Alemayehu



Re: libkface

2014-09-30 Thread Gilles Caulier
2014-09-30 11:57 GMT+02:00 David Edmundson :
>
>
> On Tue, Sep 30, 2014 at 11:38 AM, Gilles Caulier 
> wrote:
>>
>> 2014-09-30 11:28 GMT+02:00 Martin Klapetek :
>> > On Tue, Sep 30, 2014 at 7:44 AM, Gilles Caulier
>> > 
>> > wrote:
>> >>
>> >> 2014-09-30 3:06 GMT+02:00 Vishesh Handa :
>> >> > Hey Tobias
>> >> >
>> >> > Some comments about the code -
>> >> >
>> >> > 1. The code seems to be licensed under GPL. In order to make it into
>> >> > a
>> >> > framework, it will need to be re-licensed. This library seems like an
>> >> > ideal
>> >> > candidate for becoming a framework.
>> >>
>> >> libkface have been writted in same way than libkipi, libkexiv2, and
>> >> libkdcraw, already in KDEGraphics.
>> >
>> >
>>
>> Hi Martin,
>>
>> > The thing is - if libkface is set to become a framework
>
>
> Right now it's only targeting a lib in kdegraphics, not a framework.
>

yes, this is the first stage.

Note : libkexiv2, libkdcraw, and libkipi are not yet ported to KF5,
but it's planed in the future...

Gilles Caulier


Re: libkface

2014-09-30 Thread David Edmundson
On Tue, Sep 30, 2014 at 11:38 AM, Gilles Caulier 
wrote:

> 2014-09-30 11:28 GMT+02:00 Martin Klapetek :
> > On Tue, Sep 30, 2014 at 7:44 AM, Gilles Caulier <
> caulier.gil...@gmail.com>
> > wrote:
> >>
> >> 2014-09-30 3:06 GMT+02:00 Vishesh Handa :
> >> > Hey Tobias
> >> >
> >> > Some comments about the code -
> >> >
> >> > 1. The code seems to be licensed under GPL. In order to make it into a
> >> > framework, it will need to be re-licensed. This library seems like an
> >> > ideal
> >> > candidate for becoming a framework.
> >>
> >> libkface have been writted in same way than libkipi, libkexiv2, and
> >> libkdcraw, already in KDEGraphics.
> >
> >
>
> Hi Martin,
>
> > The thing is - if libkface is set to become a framework


Right now it's only targeting a lib in kdegraphics, not a framework.


Re: libkface

2014-09-30 Thread David Edmundson
On Sat, Sep 27, 2014 at 1:39 PM, Gilles Caulier 
wrote:

> 2014-09-27 12:59 GMT+02:00 David Edmundson :
> > Please make sure to add license headers on all files in libkface before
> > release. There seem to be a lot missing.
>
> Hum, which one ?
>

My bad, the script licensecheck is failing on the BSD licenses and comes up
as unknown.
Opening by hand it looks OK.



>
> I review all code in libkface in-deep while 2 weeks this summer and i
> think to not have forget something here...
>
> Gilles Caulier
>


Re: libkface

2014-09-30 Thread Gilles Caulier
2014-09-30 11:28 GMT+02:00 Martin Klapetek :
> On Tue, Sep 30, 2014 at 7:44 AM, Gilles Caulier 
> wrote:
>>
>> 2014-09-30 3:06 GMT+02:00 Vishesh Handa :
>> > Hey Tobias
>> >
>> > Some comments about the code -
>> >
>> > 1. The code seems to be licensed under GPL. In order to make it into a
>> > framework, it will need to be re-licensed. This library seems like an
>> > ideal
>> > candidate for becoming a framework.
>>
>> libkface have been writted in same way than libkipi, libkexiv2, and
>> libkdcraw, already in KDEGraphics.
>
>

Hi Martin,

> The thing is - if libkface is set to become a framework and be part of the
> KDE Frameworks effort, it has to follow KDE Frameworks policies and rules.
> One of those is that the code is licensed under LGPLv2.1+ (I think, someone
> correct me if I'm wrong). So libkface would have to be relicensed.
> Unfortunately same for the other listed libraries if they should become
> frameworks. And we would most certainly welcome that ;)

Sure, libkipi, libkexiv2, and libkdcraw will be ported to KF5, as they
are already included into kdegraphics.

I don't have any objection to re-license these libs...

Gilles


Re: libkface

2014-09-30 Thread Martin Klapetek
On Tue, Sep 30, 2014 at 7:44 AM, Gilles Caulier 
wrote:

> 2014-09-30 3:06 GMT+02:00 Vishesh Handa :
> > Hey Tobias
> >
> > Some comments about the code -
> >
> > 1. The code seems to be licensed under GPL. In order to make it into a
> > framework, it will need to be re-licensed. This library seems like an
> ideal
> > candidate for becoming a framework.
>
> libkface have been writted in same way than libkipi, libkexiv2, and
> libkdcraw, already in KDEGraphics.
>

The thing is - if libkface is set to become a framework and be part of the
KDE Frameworks effort, it has to follow KDE Frameworks policies and rules.
One of those is that the code is licensed under LGPLv2.1+ (I think, someone
correct me if I'm wrong). So libkface would have to be relicensed.
Unfortunately same for the other listed libraries if they should become
frameworks. And we would most certainly welcome that ;)

>
> > 2. The copyright header seems to say "Part of the Digikam Project". You
> may
> > want to change that.
>
> Idem here. libkface follow exactly the same way than libkipi,
> libkexiv2, libkdcraw.
>

Personally I see nothing wrong with the "Part of the Digikam Project"; if
that framework is being developed as part of Digikam project, then why not
(we still have many files with "this is part of kdelibs project" btw).


> >
> > 4. The coding style uses seems a little unorthdox. Could you perhaps add
> a
> > link to where one can know what style is being followed? Maybe this
> could go
> > in the README file.
>
> coding style follow instructions from digiKam project :
>

That's another policy of the Frameworks - we try to have a consistent
coding style all over Frameworks, that means that libkface would have to
start following the same style in order to be included in Frameworks.

Cheers
-- 
Martin Klapetek | KDE Developer


Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla

2014-09-30 Thread Jan Kundrát

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



drkonqi/bugzillalib.h


These are never saved on disk, right?

I don't think that this makes much sense given that the rest of the stack 
will happily use regular QString/QByteArray/QIODevice for actual network IO.



drkonqi/bugzillalib.h


`const QString &`



drkonqi/bugzillalib.cpp


It's customary to use smallCaps for these identifiers



drkonqi/bugzillalib.cpp


I would suggest a QLatin1String here, but that's me



drkonqi/bugzillalib.cpp


10 is a default, so I'm not convinced it's worth stating it manually



drkonqi/bugzillalib.cpp


While this code is correct, I dislike the fact that `currentVersion[3]` and 
`part > 2` are related together.

Yes, this one is longer, but also safer:

if (part >= sizeof(currentVersion)/sizeof(*currentVersion)) {
  // ...



drkonqi/bugzillalib.cpp


Same here -- that `3` is a magic number.



drkonqi/bugzillalib.cpp


It would be much cleaner IMHO if there was a single 
map/associative_array/... mapping the versions to feature set. The proposed way 
is IMHO quite fragile as the code relies on the fact that the `security` and 
`Versions` share the same length, yet this is not checked anywhere.



drkonqi/bugzillalib.cpp


switch statement to make it obvious that we're checking an enum and want to 
react to each and every option?



drkonqi/bugzillalib.cpp


What guarantees that there's always result[0]?



drkonqi/bugzillalib.cpp


This leaks authentication data to an on-disk log.


- Jan Kundrát


On Sept. 30, 2014, 7:30 a.m., Ian Wadham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120431/
> ---
> 
> (Updated Sept. 30, 2014, 7:30 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Runtime and Ben Cooksley.
> 
> 
> Bugs: 337742
> http://bugs.kde.org/show_bug.cgi?id=337742
> 
> 
> Repository: kde-runtime
> 
> 
> Description
> ---
> 
> When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security 
> method used by Bugzilla changed from cookies to tokens that had to be 
> supplied as parameters with every secure remote-procedure call. Further 
> changes to security methods have been announced by Bugzilla and are 
> documented for unstable 4.5.x versions of Bugzilla software. Tokens will be 
> deprecated and then discontinued. When this happens, Dr Konqi will need to 
> supply a user-login name and a password with every secure remote-procedure 
> call. Furthermore, the traditional "User.login" call presently used by Dr 
> Konqi will be deprecated and discontinued.
> 
> This patch fixes the tokens problem, which has given rise to several bug 
> reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also 
> provides for automatic switching to passwords-only security as and when the 
> Bugzilla version changes again. This uses
> a general data-driven approach which can be easily updated, ahead of time, 
> next time Bugzilla announces a change that affects Dr Konqi, whether it be in 
> security methods or some other feature.
> 
> NOTES:
> 1. This patch is intended to be forward-portable to Frameworks/KF5, but I 
> work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do 
> the porting and testing. So could someone else please do it?
> 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses 
> the tokens issue only, but it should be reviewed and shipped as a matter of 
> urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 
> 4.14 being due for tagging on Thursday, 9 October. That will leave more time 
> for this review (120431) of my more long-term and more general patch.
> 3. The passwords-only part of my patch is currently storing the password in 
> clear. Suggestions re encryption are welcomed --- or the code could be 
> changed to make use of KWalletD mandatory (but that might not be fully 
> portable to all platforms).
> 4. When the Bugzilla call "User.login" is

Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla

2014-09-30 Thread René J . V . Bertin

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


Great, it's been ... bugging me that the bug reporter hasn't been able to do 
its final step since quite some time already.

Not to diminish the importance of your work, but someone really ought to 
provide a means (or document how) to upload a finished bug report without 
copy/pasting it in bits and pieces. An import button and a shortcut to the last 
page when launching the reporter manually would be all that's needed once the 
login step is moved to that page.
After all there are other reasons why the final upload in the reported failed.

> The passwords-only part of my patch is currently storing the password in 
> clear. Suggestions re encryption are welcomed --- or the code could be 
> changed to make use of KWalletD mandatory (but that might not be fully 
> portable to all platforms).

Is that really a concern? I suppose no one uses his or her bank account 
password on a bug reporting site and it's not like open source software will 
contain industrial secrets either. Make sure the sensitive information is not 
stored in static memory, make sure regular builds will not contain debug info, 
and it ought to be hard enough to obtain a password from a memory image. The 
reporter would normally not be running for a long time anyway.
Also, while not an expert on the matter I'm guessing that whatever argument you 
can forward why caching creds in unencrypted RAM is a bad idea can be extended 
to every kind of purely in-memory encryption. (Including kwallet if the user 
doesn't set it to lock after each transaction.)

- René J.V. Bertin


On Sept. 30, 2014, 9:30 a.m., Ian Wadham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120431/
> ---
> 
> (Updated Sept. 30, 2014, 9:30 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Runtime and Ben Cooksley.
> 
> 
> Bugs: 337742
> http://bugs.kde.org/show_bug.cgi?id=337742
> 
> 
> Repository: kde-runtime
> 
> 
> Description
> ---
> 
> When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security 
> method used by Bugzilla changed from cookies to tokens that had to be 
> supplied as parameters with every secure remote-procedure call. Further 
> changes to security methods have been announced by Bugzilla and are 
> documented for unstable 4.5.x versions of Bugzilla software. Tokens will be 
> deprecated and then discontinued. When this happens, Dr Konqi will need to 
> supply a user-login name and a password with every secure remote-procedure 
> call. Furthermore, the traditional "User.login" call presently used by Dr 
> Konqi will be deprecated and discontinued.
> 
> This patch fixes the tokens problem, which has given rise to several bug 
> reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also 
> provides for automatic switching to passwords-only security as and when the 
> Bugzilla version changes again. This uses
> a general data-driven approach which can be easily updated, ahead of time, 
> next time Bugzilla announces a change that affects Dr Konqi, whether it be in 
> security methods or some other feature.
> 
> NOTES:
> 1. This patch is intended to be forward-portable to Frameworks/KF5, but I 
> work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do 
> the porting and testing. So could someone else please do it?
> 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses 
> the tokens issue only, but it should be reviewed and shipped as a matter of 
> urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 
> 4.14 being due for tagging on Thursday, 9 October. That will leave more time 
> for this review (120431) of my more long-term and more general patch.
> 3. The passwords-only part of my patch is currently storing the password in 
> clear. Suggestions re encryption are welcomed --- or the code could be 
> changed to make use of KWalletD mandatory (but that might not be fully 
> portable to all platforms).
> 4. When the Bugzilla call "User.login" is discontinued, some re-sequencing of 
> the flow of KAssistantDialog pages will be needed. I have not attempted to do 
> that at this stage. Probably the entry of the user name and password should 
> be delayed until the report has been accepted by the Dr Konqi logic and it is 
> just about to be sent to bugs.kde.org or attached to an existing bug report.
> 
> REFERENCES:
> http://www.bugzilla.org/docs/
> http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN
>  Bugzilla 4.5.x (future) API doco re security
> http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebSe

Re: libkface

2014-09-30 Thread Gilles Caulier
2014-09-30 7:44 GMT+02:00 Gilles Caulier :
> 2014-09-30 3:06 GMT+02:00 Vishesh Handa :
>> Hey Tobias
>>
>> Some comments about the code -
>>
>> 1. The code seems to be licensed under GPL. In order to make it into a
>> framework, it will need to be re-licensed. This library seems like an ideal
>> candidate for becoming a framework.
>
> libkface have been writted in same way than libkipi, libkexiv2, and
> libkdcraw, already in KDEGraphics.
>
> https://projects.kde.org/projects/kde/kdegraphics/libs/libkipi
> https://projects.kde.org/projects/kde/kdegraphics/libs/libkexiv2
> https://projects.kde.org/projects/kde/kdegraphics/libs/libkdcraw
>
>>
>> 2. The copyright header seems to say "Part of the Digikam Project". You may
>> want to change that.
>
> Idem here. libkface follow exactly the same way than libkipi,
> libkexiv2, libkdcraw.
>
>>
>> 3. There is an empty TODO file
>
> yes, this need to be filled.
>
>>
>> 4. The coding style uses seems a little unorthdox. Could you perhaps add a
>> link to where one can know what style is being followed? Maybe this could go
>> in the README file.
>
> coding style follow instructions from digiKam project :
>
> https://projects.kde.org/projects/extragear/graphics/digikam/repository/revisions/master/entry/HACKING
>
> It's the same coding style that libkexiv2, libkipi, and libkdcraw.
>
>>
>> 5. Identity ABI - The Identity class seems to be missing a d pointer.

New bugzilla entry created with patch :

https://bugs.kde.org/show_bug.cgi?id=339524

Tobias, this need to be tested. Also, PhotoAlbum code need to be adapted.

Gilles


Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla

2014-09-30 Thread Ian Wadham

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

Review request for KDE Software on Mac OS X, KDE Runtime and Ben Cooksley.


Bugs: 337742
http://bugs.kde.org/show_bug.cgi?id=337742


Repository: kde-runtime


Description
---

When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security 
method used by Bugzilla changed from cookies to tokens that had to be supplied 
as parameters with every secure remote-procedure call. Further changes to 
security methods have been announced by Bugzilla and are documented for 
unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and 
then discontinued. When this happens, Dr Konqi will need to supply a user-login 
name and a password with every secure remote-procedure call. Furthermore, the 
traditional "User.login" call presently used by Dr Konqi will be deprecated and 
discontinued.

This patch fixes the tokens problem, which has given rise to several bug 
reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also 
provides for automatic switching to passwords-only security as and when the 
Bugzilla version changes again. This uses
a general data-driven approach which can be easily updated, ahead of time, next 
time Bugzilla announces a change that affects Dr Konqi, whether it be in 
security methods or some other feature.

NOTES:
1. This patch is intended to be forward-portable to Frameworks/KF5, but I work 
on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the 
porting and testing. So could someone else please do it?
2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses 
the tokens issue only, but it should be reviewed and shipped as a matter of 
urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 
being due for tagging on Thursday, 9 October. That will leave more time for 
this review (120431) of my more long-term and more general patch.
3. The passwords-only part of my patch is currently storing the password in 
clear. Suggestions re encryption are welcomed --- or the code could be changed 
to make use of KWalletD mandatory (but that might not be fully portable to all 
platforms).
4. When the Bugzilla call "User.login" is discontinued, some re-sequencing of 
the flow of KAssistantDialog pages will be needed. I have not attempted to do 
that at this stage. Probably the entry of the user name and password should be 
delayed until the report has been accepted by the Dr Konqi logic and it is just 
about to be sent to bugs.kde.org or attached to an existing bug report.

REFERENCES:
http://www.bugzilla.org/docs/
http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN
 Bugzilla 4.5.x (future) API doco re security
http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN
 Bugzilla 4.4.5 (current) API doco re security
http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login
 User.login will be DEPRECATED in 4.5.x


Diffs
-

  drkonqi/bugzillalib.h 570169b 
  drkonqi/bugzillalib.cpp f74753c 
  drkonqi/reportassistantpages_bugzilla.h b7af5b8 
  drkonqi/reportassistantpages_bugzilla.cpp 22183f0 

Diff: https://git.reviewboard.kde.org/r/120431/diff/


Testing
---

Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime 
repository.

Tested a range of version numbers (see commented-out test data) against a range 
of 5 or 6 hypothetical and real Bugzilla versions at which things could or will 
change. This was to test the basic version-checking and feature-choosing 
algorithm.

Tested submitting both full reports and attached reports, using both the token 
method and the passwords-only method.

Also tested with KWalletD supplying the username and password on Dr Konqi's 
login dialog.


Thanks,

Ian Wadham