<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/104631/">http://git.reviewboard.kde.org/r/104631/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 26th, 2012, 5:09 p.m., <b>Albert Astals Cid</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;">If i understand you correctly you are suggesting to create a bug (option that does nothing)?
Doesn't make much sense.</pre>
</blockquote>
<p>On May 1st, 2012, 5:05 p.m., <b>Dawit Alemayehu</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;">Huh ? I do not follow. By "option that does nothing" you mean this change by itself does nothing that is correct. But that is true of every option in that dialog as well. Moreover, it is a well known fact that you cannot post a patch for different components in reviewboard. Anyhow, the option will most definitely be used by kwebkitpart. Whether or not some body implements support for it in khtml is a question I cannot answer. That is what I meant.</pre>
</blockquote>
<p>On May 4th, 2012, 9:39 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;">Do you have the kwebkitpart patch ready?
I suppose it'll be easy to "apply" to khtml as well.</pre>
</blockquote>
<p>On May 4th, 2012, 10:14 p.m., <b>Albert Astals Cid</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 are suggesting to add an option that does nothing in our default html rendering engine. That is adding a bug. The fact that you know it and don't care about it makes me sad.</pre>
</blockquote>
<p>On May 9th, 2012, 8:19 p.m., <b>Dawit Alemayehu</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;">@David: Yes I have a patch for kwebkitpart, but unlike in khtml adding support for this in kwebkitpart required a very small change in one place in addition to adding code to read the config option itself in webkitettings.{h|cpp}. That does not seem to be the case in khtml. It is a little bit more invovled.
@Albert: Really ?? So how exactly is another browser engine supposed to provide configuration option to the user if it is supposed to be embedded into Konqueror ? Why would a developer working on a separate browsing engine be forced to implement any functionality into khtml ? Would that requirement apply the other way around ? The last I checked, this is a konqueror setting. Whether a specific browser engine supports it or not is up to that browser engine.Just for the record I almost always go out of my way to implement things in khtml ; especially when I factor out features that are common to both engines. In this case, I neither have the time nor a complete grasp of how the KWallet integration works in khtml. As I stated above the change is not in a single isolated location like it is in kwebkitpart. Feel free to do a grep in khtml and see for yourself. I can always add the set/get functions to access the option in KHTMLSettings, but what use would that be if it is not implemented.
Anyhow, I digress. If there are objections, I will simply move this feature into the webkit config module I will have to create down the road.</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;">"So how exactly is another browser engine supposed to provide configuration option to the user if it is supposed to be embedded into Konqueror ?"
Having it's own engine-only kcm for it's engine-specific options?
"Why would a developer working on a separate browsing engine be forced to implement any functionality into khtml ?"
When did i say that?
"Would that requirement apply the other way around ?"
If you want to use the "general" apply to all engines options page, of course.
You probably don't have any bit of user mentallity left in your head, because it's pretty obvious that adding a configuration option to "web rendering configuration" that won't work with our default rendering engine it's bad usability.
But hey, since David promised to implement this for khtml, go ahead, don't let me block progress.
</pre>
<br />
<p>- Albert</p>
<br />
<p>On April 26th, 2012, 3:48 a.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, kdelibs and David Faure.</div>
<div>By Dawit Alemayehu.</div>
<p style="color: grey;"><i>Updated April 26, 2012, 3:48 a.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;">A patch to make that provides an option to disable the "offer to save website passwords". Note that for this patch to take effect this option will have to be used at at the browser engine level. Those patches are separate to this one. This is just the GUI configuration.</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>konqueror/settings/konqhtml/htmlopts.h <span style="color: grey">(69f36d8)</span></li>
<li>konqueror/settings/konqhtml/htmlopts.cpp <span style="color: grey">(e5d6deb)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/104631/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>
<div>
<a href="http://git.reviewboard.kde.org/r/104631/s/544/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/04/26/konq_save_password_config_400x100.png" style="border: 1px black solid;" alt="Option for disabling KWallet integration" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>