Re: Review Request: Added option for disabling the offer to save website passwords in Konqueror
On April 26, 2012, 5:09 p.m., Albert Astals Cid wrote:
If i understand you correctly you are suggesting to create a bug (option
that does nothing)?
Doesn't make much sense.
Dawit Alemayehu wrote:
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.
David Faure wrote:
Do you have the kwebkitpart patch ready?
I suppose it'll be easy to apply to khtml as well.
Albert Astals Cid wrote:
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.
Dawit Alemayehu wrote:
@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.
Albert Astals Cid wrote:
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.
Martin Tobias Holmedahl Sandsmark wrote:
I just briefly looked at how complex it would be to implement in KHTML,
and it seems to be enough with this three-line patch?
diff --git a/khtml/ui/passwordbar/storepassbar.cpp
b/khtml/ui/passwordbar/storepassbar.cpp
index 3f5c392..08d79a9 100644
--- a/khtml/ui/passwordbar/storepassbar.cpp
+++ b/khtml/ui/passwordbar/storepassbar.cpp
@@ -80,6 +80,10 @@ StorePass::~StorePass()
void StorePass::saveLoginInformation(const QString host, const QString
key,
const QMapQString, QString walletMap)
{
+ KConfigGroup config( KGlobal::config(), HTML Settings );
+ if (!config.readEntrybool(OfferToSaveWebsitePassword, true))
+return;
+
m_host = host;
m_key = key;
m_walletMap = walletMap;
Dawit Alemayehu wrote:
@Martin: Great. Thanks for the patch.
KHTML patch tested, it works.
- David
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104631/#review12934
---
On April 26, 2012, 3:48 a.m., Dawit Alemayehu wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104631/
---
(Updated April 26, 2012, 3:48 a.m.)
Review request
Re: Review Request: Added option for disabling the offer to save website passwords in Konqueror
On April 26, 2012, 5:09 p.m., Albert Astals Cid wrote:
If i understand you correctly you are suggesting to create a bug (option
that does nothing)?
Doesn't make much sense.
Dawit Alemayehu wrote:
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.
David Faure wrote:
Do you have the kwebkitpart patch ready?
I suppose it'll be easy to apply to khtml as well.
Albert Astals Cid wrote:
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.
Dawit Alemayehu wrote:
@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.
Albert Astals Cid wrote:
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.
Martin Tobias Holmedahl Sandsmark wrote:
I just briefly looked at how complex it would be to implement in KHTML,
and it seems to be enough with this three-line patch?
diff --git a/khtml/ui/passwordbar/storepassbar.cpp
b/khtml/ui/passwordbar/storepassbar.cpp
index 3f5c392..08d79a9 100644
--- a/khtml/ui/passwordbar/storepassbar.cpp
+++ b/khtml/ui/passwordbar/storepassbar.cpp
@@ -80,6 +80,10 @@ StorePass::~StorePass()
void StorePass::saveLoginInformation(const QString host, const QString
key,
const QMapQString, QString walletMap)
{
+ KConfigGroup config( KGlobal::config(), HTML Settings );
+ if (!config.readEntrybool(OfferToSaveWebsitePassword, true))
+return;
+
m_host = host;
m_key = key;
m_walletMap = walletMap;
Dawit Alemayehu wrote:
@Martin: Great. Thanks for the patch.
David Faure wrote:
KHTML patch tested, it works.
Yes, I tested it too and it worked from me as well. I did not commit the change
because the option is really only useful in 4.9.0 so I was waiting until 4.8.4
is tagged and released at the end of this month.
- Dawit
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104631/#review12934
---
On April 26, 2012, 3:48 a.m., Dawit Alemayehu wrote:
Re: Review Request: Added option for disabling the offer to save website passwords in Konqueror
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104631/#review13952 --- This review has been submitted with commit bf7114bfc64b97257e8dc69af58ccaa17abf690f by Dawit Alemayehu to branch master. - Commit Hook On April 26, 2012, 3:48 a.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104631/ --- (Updated April 26, 2012, 3:48 a.m.) Review request for KDE Base Apps, kdelibs and David Faure. Description --- 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. Diffs - konqueror/settings/konqhtml/htmlopts.h 69f36d8 konqueror/settings/konqhtml/htmlopts.cpp e5d6deb Diff: http://git.reviewboard.kde.org/r/104631/diff/ Testing --- Screenshots --- Option for disabling KWallet integration http://git.reviewboard.kde.org/r/104631/s/544/ Thanks, Dawit Alemayehu
Re: Review Request: Added option for disabling the offer to save website passwords in Konqueror
On April 26, 2012, 5:09 p.m., Albert Astals Cid wrote:
If i understand you correctly you are suggesting to create a bug (option
that does nothing)?
Doesn't make much sense.
Dawit Alemayehu wrote:
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.
David Faure wrote:
Do you have the kwebkitpart patch ready?
I suppose it'll be easy to apply to khtml as well.
Albert Astals Cid wrote:
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.
Dawit Alemayehu wrote:
@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.
Albert Astals Cid wrote:
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.
Martin Tobias Holmedahl Sandsmark wrote:
I just briefly looked at how complex it would be to implement in KHTML,
and it seems to be enough with this three-line patch?
diff --git a/khtml/ui/passwordbar/storepassbar.cpp
b/khtml/ui/passwordbar/storepassbar.cpp
index 3f5c392..08d79a9 100644
--- a/khtml/ui/passwordbar/storepassbar.cpp
+++ b/khtml/ui/passwordbar/storepassbar.cpp
@@ -80,6 +80,10 @@ StorePass::~StorePass()
void StorePass::saveLoginInformation(const QString host, const QString
key,
const QMapQString, QString walletMap)
{
+ KConfigGroup config( KGlobal::config(), HTML Settings );
+ if (!config.readEntrybool(OfferToSaveWebsitePassword, true))
+return;
+
m_host = host;
m_key = key;
m_walletMap = walletMap;
@Martin: Great. Thanks for the patch.
- Dawit
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104631/#review12934
---
On April 26, 2012, 3:48 a.m., Dawit Alemayehu wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104631/
---
(Updated April 26, 2012, 3:48 a.m.)
Review request for KDE Base Apps, kdelibs and David Faure.
Description
Re: Review Request: Added option for disabling the offer to save website passwords in Konqueror
On April 26, 2012, 5:09 p.m., Albert Astals Cid wrote:
If i understand you correctly you are suggesting to create a bug (option
that does nothing)?
Doesn't make much sense.
Dawit Alemayehu wrote:
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.
David Faure wrote:
Do you have the kwebkitpart patch ready?
I suppose it'll be easy to apply to khtml as well.
Albert Astals Cid wrote:
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.
Dawit Alemayehu wrote:
@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.
Albert Astals Cid wrote:
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.
I just briefly looked at how complex it would be to implement in KHTML, and it
seems to be enough with this three-line patch?
diff --git a/khtml/ui/passwordbar/storepassbar.cpp
b/khtml/ui/passwordbar/storepassbar.cpp
index 3f5c392..08d79a9 100644
--- a/khtml/ui/passwordbar/storepassbar.cpp
+++ b/khtml/ui/passwordbar/storepassbar.cpp
@@ -80,6 +80,10 @@ StorePass::~StorePass()
void StorePass::saveLoginInformation(const QString host, const QString key,
const QMapQString, QString walletMap)
{
+ KConfigGroup config( KGlobal::config(), HTML Settings );
+ if (!config.readEntrybool(OfferToSaveWebsitePassword, true))
+return;
+
m_host = host;
m_key = key;
m_walletMap = walletMap;
- Martin Tobias Holmedahl
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104631/#review12934
---
On April 26, 2012, 3:48 a.m., Dawit Alemayehu wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104631/
---
(Updated April 26, 2012, 3:48 a.m.)
Review request for KDE Base Apps, kdelibs and David Faure.
Description
---
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
Re: Review Request: Added option for disabling the offer to save website passwords in Konqueror
On April 26, 2012, 5:09 p.m., Albert Astals Cid wrote:
If i understand you correctly you are suggesting to create a bug (option
that does nothing)?
Doesn't make much sense.
Dawit Alemayehu wrote:
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.
David Faure wrote:
Do you have the kwebkitpart patch ready?
I suppose it'll be easy to apply to khtml as well.
Albert Astals Cid wrote:
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.
Dawit Alemayehu wrote:
@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.
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.
- Albert
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104631/#review12934
---
On April 26, 2012, 3:48 a.m., Dawit Alemayehu wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104631/
---
(Updated April 26, 2012, 3:48 a.m.)
Review request for KDE Base Apps, kdelibs and David Faure.
Description
---
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.
Diffs
-
konqueror/settings/konqhtml/htmlopts.h 69f36d8
konqueror/settings/konqhtml/htmlopts.cpp e5d6deb
Diff: http://git.reviewboard.kde.org/r/104631/diff/
Testing
---
Screenshots
---
Option for disabling KWallet integration
http://git.reviewboard.kde.org/r/104631/s/544/
Thanks,
Dawit Alemayehu
Re: Review Request: Added option for disabling the offer to save website passwords in Konqueror
On Sunday, May 13, 2012 12:20:00 Albert Astals Cid wrote: You probably don't have any bit of user mentallity left in your head, I think everybody is served better by discussions that do not engage in personal attacks. Let us please try to keep it respectful and technical, and avoid ad-hominem attacks. That keeps k-c-d a more friendly place to everyone. Respect is an important basis for making progress in a collaborative way. Thanks, -- sebas http://www.kde.org | http://vizZzion.org | GPG Key ID: 9119 0EF9
Re: Review Request: Added option for disabling the offer to save website passwords in Konqueror
El Diumenge, 13 de maig de 2012, a les 14:27:37, Sebastian Kügler va escriure: On Sunday, May 13, 2012 12:20:00 Albert Astals Cid wrote: You probably don't have any bit of user mentallity left in your head, I think everybody is served better by discussions that do not engage in personal attacks. This was in no way a personal attack. You'll realize when I do a personal attack. Let us please try to keep it respectful and technical I have been respectful and technical. and avoid ad-hominem attacks. No ad-hominem attack happened, an ad-hominem attack would be saying Don't listen to him, he is a communist, since being communist or not does not have any effect on his technical hability, but might put people against him. Respect is an important basis for making progress in a collaborative way. Sure it is. Cheers, Albert Thanks,
Re: Review Request: Added option for disabling the offer to save website passwords in Konqueror
you probably wanted to point out that dawit ignored the perception of a typical user when introducing this checkbox, but it actually could be read as a disqualifying assumption about the condition of his mind. latter would indeed be ad hominem and not ad res, but ultimatily what we have here is just called friction (many ppl. from different cultures talking in a foreign language) so i suggest we just leave it to this and drop that discussion, for the touched webkit/khtml item is rather prone to drag it down a dirty path by less professional followers of k-c-d.
Re: Review Request: Added option for disabling the offer to save website passwords in Konqueror
On April 26, 2012, 5:09 p.m., Albert Astals Cid wrote: If i understand you correctly you are suggesting to create a bug (option that does nothing)? Doesn't make much sense. Dawit Alemayehu wrote: 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. Do you have the kwebkitpart patch ready? I suppose it'll be easy to apply to khtml as well. - David --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104631/#review12934 --- On April 26, 2012, 3:48 a.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104631/ --- (Updated April 26, 2012, 3:48 a.m.) Review request for KDE Base Apps, kdelibs and David Faure. Description --- 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. Diffs - konqueror/settings/konqhtml/htmlopts.h 69f36d8 konqueror/settings/konqhtml/htmlopts.cpp e5d6deb Diff: http://git.reviewboard.kde.org/r/104631/diff/ Testing --- Screenshots --- Option for disabling KWallet integration http://git.reviewboard.kde.org/r/104631/s/544/ Thanks, Dawit Alemayehu
Re: Review Request: Added option for disabling the offer to save website passwords in Konqueror
On April 26, 2012, 5:09 p.m., Albert Astals Cid wrote: If i understand you correctly you are suggesting to create a bug (option that does nothing)? Doesn't make much sense. Dawit Alemayehu wrote: 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. David Faure wrote: Do you have the kwebkitpart patch ready? I suppose it'll be easy to apply to khtml as well. 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. - Albert --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104631/#review12934 --- On April 26, 2012, 3:48 a.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104631/ --- (Updated April 26, 2012, 3:48 a.m.) Review request for KDE Base Apps, kdelibs and David Faure. Description --- 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. Diffs - konqueror/settings/konqhtml/htmlopts.h 69f36d8 konqueror/settings/konqhtml/htmlopts.cpp e5d6deb Diff: http://git.reviewboard.kde.org/r/104631/diff/ Testing --- Screenshots --- Option for disabling KWallet integration http://git.reviewboard.kde.org/r/104631/s/544/ Thanks, Dawit Alemayehu
Re: Review Request: Added option for disabling the offer to save website passwords in Konqueror
On April 26, 2012, 5:09 p.m., Albert Astals Cid wrote: If i understand you correctly you are suggesting to create a bug (option that does nothing)? Doesn't make much sense. 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. - Dawit --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104631/#review12934 --- On April 26, 2012, 3:48 a.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104631/ --- (Updated April 26, 2012, 3:48 a.m.) Review request for KDE Base Apps, kdelibs and David Faure. Description --- 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. Diffs - konqueror/settings/konqhtml/htmlopts.h 69f36d8 konqueror/settings/konqhtml/htmlopts.cpp e5d6deb Diff: http://git.reviewboard.kde.org/r/104631/diff/ Testing --- Screenshots --- Option for disabling KWallet integration http://git.reviewboard.kde.org/r/104631/s/544/ Thanks, Dawit Alemayehu
Re: Review Request: Added option for disabling the offer to save website passwords in Konqueror
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104631/ --- (Updated April 26, 2012, 3:48 a.m.) Review request for KDE Base Apps, kdelibs and David Faure. Changes --- Added a screen shot of the config option. If there are no objections, I am going to commit this in a couple of days. Hopefully the khtml guys will implement this option there as well. Description --- 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. Diffs - konqueror/settings/konqhtml/htmlopts.h 69f36d8 konqueror/settings/konqhtml/htmlopts.cpp e5d6deb Diff: http://git.reviewboard.kde.org/r/104631/diff/ Testing --- Screenshots (updated) --- Option for disabling KWallet integration http://git.reviewboard.kde.org/r/104631/s/544/ Thanks, Dawit Alemayehu
