<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/100374/">http://git.reviewboard.kde.org/r/100374/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 17th, 2011, 10:18 a.m., <b>Andrea Diamantini</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;">I'm not completely satisfied from this patch. Please, give people time to review code. In example, I'm a bit dubious about the use of the "showDeveloperTools" bit. Shouldn't we use the WebKit "developerExtras" flag instead?</pre>
</blockquote>
<p>On January 17th, 2011, 2:35 p.m., <b>Benjamin Poulain</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;">Just trying to help. :(
If you want to review every patch I think that will not scale. Worse case we can just revert it.
I think it makes sense to have it separated from developerExtras because it aslo change the "view page source" action. At the moment, WebKit's developerExtras can be enabled in all cases in my opinion, so you can enable showDeveloperTools without having to reload all the pages.</pre>
</blockquote>
<p>On January 17th, 2011, 4:15 p.m., <b>Andrea Diamantini</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;">You did it well, Benjamin.
I don't absolutely want to review all patches to be merged, in particular those just debated. I have had just this question about the developer flag and I thought the easiest way was to reopen the request.
In fact we have here IMHO same strange behaviors. Let me give you some examples: with showDeveloperTools disabled we don't have dev options on context menu (good) but we have them on rekonq menu (in the development submenu. Good?) . Activating one option from there, WebKit developerExtras is automatically enabled, so that you can see code / inspector / net analyzer, but rekonq showDeveloperTools will remain disabled.
I think this opens some options and I thought about this in the morning but I couldn't decide what is better:
1) use just WebKit developerExtras flag. This will be enabled/disabled on the settings menu and while activating the developer options.
2) remove also developer submenu from rekonq menu when showDeveloperTools is disabled.
3) maintain code as is, adding extrachecks during webview context menu creation to show dev tools when either showDeveloperTools and developerExtras are set.
In case 2) & 3), I think the setting should be removed from the webkit group and added into a misc one.
What do you guys think?</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;">I use the main menu so little that I actually never saw there was developer tools there as well. You are right, they should be disabled there as well. :)
I am not sure it is worth disabling developerExtras ever. The overhead is low on Desktop. Enabling it and disabling it at runtime will require reload of pages to have everything working.</pre>
<br />
<p>- Benjamin</p>
<br />
<p>On January 14th, 2011, 10:08 p.m., Felix Rohrbach wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.orgrb/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 rekonq.</div>
<div>By Felix Rohrbach.</div>
<p style="color: grey;"><i>Updated Jan. 14, 2011, 10:08 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;">This patch contains the changes to the context menu which we decided in our meeting:
1. Hide new tab entry if it is not necessary.
2. Print and Search action: I removed the print action. The search action is now in the "search with" submenu if some text is selected.
3. Hide the development menu entries and add an option to show them.
Clone: git://anongit.kde.org/clones/rekonq/felixr/rekonq
Branch: meetingMenuChanges
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/mainwindow.h <span style="color: grey">(7dfc186)</span></li>
<li>src/mainwindow.cpp <span style="color: grey">(599295f)</span></li>
<li>src/rekonq.kcfg <span style="color: grey">(cda76d8)</span></li>
<li>src/settings/settings_webkit.ui <span style="color: grey">(58fbe45)</span></li>
<li>src/webview.cpp <span style="color: grey">(49fa103)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100374/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>