Bug 196778 - web shortcuts: non-search engine are incorrectly handled
Summary: web shortcuts: non-search engine are incorrectly handled
Status: RESOLVED FIXED
Alias: None
Product: konqueror
Classification: Applications
Component: general (show other bugs)
Version: 4.3.0
Platform: openSUSE Unspecified
: NOR normal
Target Milestone: ---
Assignee: Konqueror Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-16 18:30 UTC by Maciej Pilichowski
Modified: 2012-07-28 08:16 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.9.0


Attachments
Screenshot after step 5. (94.04 KB, image/png)
2009-11-04 20:06 UTC, esigra
Details
Screenshot after step 7. (93.96 KB, image/png)
2009-11-04 20:07 UTC, esigra
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Maciej Pilichowski 2009-06-16 18:30:57 UTC
Version:            (using KDE 4.2.90)
Installed from:    SuSE RPMs

There are a lot of problem related to web shortcuts, which in short could be put -- Konqueror _prefers_ searching.
As the effect, user is intimidated to use only web shortcuts for searching, nothing else. Run Konq, go to settings, web shortcuts section:
a) "default search engine" --> it should be "default web shortcut"
b) when adding web shortcut user is reminded each time, she/he entered web shortcut, and not search engine. Respect user actions, web shortcut is not invalid entry, so please accept it without complaining
c) on the second run of Konqueror all web shortcuts which are not search shortcuts are zombies -- user cannot edit them or delete them, they are listed, but they are untouchable

Another approach would be banning non-search-shortcuts, it is ok with me, but then Konq. should behave consistently anyway:
a) keep the current label
b) forbid such entries
c) solved by (b)
d) "web shorcuts" -> "search shortcuts"
Comment 1 esigra 2009-11-04 20:06:35 UTC
Created attachment 38088 [details]
Screenshot after step 5.
Comment 2 esigra 2009-11-04 20:07:16 UTC
Created attachment 38089 [details]
Screenshot after step 7.
Comment 3 esigra 2009-11-04 20:12:11 UTC
Sorry, these screenshots were for bug #213129, which by the way seems related to the issue mentioned under c) of this bug.
Comment 4 Dawit Alemayehu 2011-10-15 05:17:45 UTC
There is no reason why you should not be able to delete any web shortcut from the list. You should be able to add, change or delete web shortcuts at any time. I tested this over and over again, even closing konqueror and starting it again and I cannot reproduce the problem. The second I select a web shortcut, the "change" and "delete" buttons are enabled. If that is not happening on your system, then there is something else going on.

Just on a hunch, what desktop style are you using ? Oxygen ? In case you dunno how to check, you can type "style" and press enter in the KRunner (ALT+F2). If you are not using the default style engine (Oxygen), does changing the style to the default engine fix your problem ?
Comment 5 Maciej Pilichowski 2011-10-15 07:38:53 UTC
I just have hunch, you didn't read original post -- to ensure I am correct, could you please attach the screenshot of Konqueror settings, "web shortcuts" section (with English language). Thank you.

For the record, I mentioned 3 problems. I will use upper-case symbols now for them to avoid confusion with original post:
A) still holds
B) still holds
C) fixed
Comment 6 Dawit Alemayehu 2011-10-15 08:09:44 UTC
(In reply to comment #5)
> I just have hunch, you didn't read original post -- to ensure I am correct,
> could you please attach the screenshot of Konqueror settings, "web shortcuts"
> section (with English language). Thank you.
> 
> For the record, I mentioned 3 problems. I will use upper-case symbols now for
> them to avoid confusion with original post:
> A) still holds
> B) still holds
> C) fixed

There is no confusion and I did read the original post. I simply chose to respond to what I thought was the bug stated in C. Since you state C is fixed concerned then let me deal with with A and B which are non-critical changes.

I have already change the label for A in my local copy and will commit that change to master since I agree with your sentiment that the label should say "Default web shortcut". I also agree the labels in the dialog used to add and change shortcuts should be changed to reflect that fact.

As far as B is concerned, I am thinking about removing that warning since it no longer makes sense. The warning was added before the web shortcuts functionality were updated to allow non-search related shortcuts. However, there is one problem with removing it now. The user will no longer be notified when {s}he really forgets to add the placeholder for a search related web shortcut.

Does that answer all your questions ?
Comment 7 Maciej Pilichowski 2011-10-15 08:18:45 UTC
Yes.

Btw. even if user forget to add query part, such "mistake" is not only not critical, but also very easy to fix. It is below typo in document (you have to print document again).

Btw.2 the definition of web shortcut says "Search URI", so this issue (D).

It would be more useful and clear, that instead of checking for query, there would be a query placeholder button

Web shortcut URI: [_____________] (oo) [insert query placeholder]

Above you have:
label
edit box
icon (oo)
button

The icon is binoculars, it is disabled until user clicks "insert query placeholder" or it is detected, there is query placeholder within URI.

Such UI would be more well, polite, or IOW -- less intrusive.

One more advantage -- no one would have to remember what is the symbol of query placeholder.
Comment 8 Dawit Alemayehu 2012-05-11 20:35:00 UTC
Git commit d7d9a5f97de9e4e078edccb5b36bad192d4aede2 by Dawit Alemayehu.
Committed on 09/05/2012 at 21:59.
Pushed by adawit into branch 'master'.

- Replaced all references of 'search' with 'web shortcuts' since web shortcuts
  are not only for doing search.

- Replaced all the WhatsThis help text with equivalent ToolTip text.

Note that this change only renames related config options, variables and GUI text.
In the future eveything, including file and class names, should be renamed to
reflect the current functionality of this plugin.
FIXED-IN: 4.9.0
GUI:

M  +1    -1    kurifilter-plugins/ikws/ikwsopts.cpp
M  +35   -39   kurifilter-plugins/ikws/ikwsopts_ui.ui
M  +7    -0    kurifilter-plugins/ikws/kuriikwsfilter.upd
M  +14   -14   kurifilter-plugins/ikws/kuriikwsfiltereng.cpp
M  +4    -4    kurifilter-plugins/ikws/kuriikwsfiltereng.h
M  +6    -6    kurifilter-plugins/ikws/searchproviderdlg.cpp
M  +11   -14   kurifilter-plugins/ikws/searchproviderdlg_ui.ui

http://commits.kde.org/kde-runtime/d7d9a5f97de9e4e078edccb5b36bad192d4aede2
Comment 9 Andrea Diamantini 2012-07-27 08:31:14 UTC
uhm... since this commit, everyone has lost his favorite and preferred engines (or web shortcut, whatever you prefer) settings.
Was't a string change enough for it?
Comment 10 Dawit Alemayehu 2012-07-27 13:58:11 UTC
(In reply to comment #9)
> uhm... since this commit, everyone has lost his favorite and preferred
> engines (or web shortcut, whatever you prefer) settings.
> Was't a string change enough for it?

And get totally confused about what is what ? IOW, the strings won't match the variables that hold the data. 

Anyhow, the actual bug you mentioned was my fault. I did not make the variable name changes everywhere as I should have. However, I have already addressed that when I run into the same problem. See https://projects.kde.org/projects/kde/kde-runtime/repository/revisions/489b8eaa49a0af70840c8ce165d731f6c24aa8c4
If you are using a version with that commit, you should no longer see this bug.
Comment 11 Andrea Diamantini 2012-07-27 14:14:51 UTC
I was meaning... why don't change just the strings in the UI, instead of the variables names?
btw, I'm using this morning KDE libs + runtime 4.9 branch. My search engine is switched to  none and and no more have preferred shortcuts. So, I don't think that commit fixes everything. Yes, setting them another time now fixes it, but no backward compatibility with 4.8, that it is a pity.
Comment 12 Dawit Alemayehu 2012-07-27 14:37:40 UTC
(In reply to comment #11)
> I was meaning... why don't change just the strings in the UI, instead of the
> variables names?
> btw, I'm using this morning KDE libs + runtime 4.9 branch. My search engine
> is switched to  none and and no more have preferred shortcuts. So, I don't
> think that commit fixes everything. Yes, setting them another time now fixes
> it, but no backward compatibility with 4.8, that it is a pity.

That should not be the case at all. Was this the first time you updated kde-runtime ?  Otherwise, this does not make any sense because I changed nothing recently. Second, we of course always provide backward compatibility for config variable name changes through the ".upd" files. It would be silly to do otherwise. You can see that change in the original commit:

https://projects.kde.org/projects/kde/kde-runtime/repository/revisions/d7d9a5f97de9e4e078edccb5b36bad192d4aede2/diff/kurifilter-plugins/ikws/kuriikwsfilter.upd

I will test it again to see if the change works correctly for those upgrading to 4.9.
Comment 13 Andrea Diamantini 2012-07-27 14:56:52 UTC
well.. it did not work here. Or better, it updated the variables just when I tried to set it again. Let me explain:
My 4.8 situation:
- search engine: google.
- favorites search engines: google, duckduckgo, wikipedia, youtube
After upgrade to 4.9
- search engine: none
- favorites search engines: none
Then I tried to set them again, but instead of setting the old ones, I set duckduckgo as default and set google, wk and youtube as favorites.
Actual situation
- search engine: duckduckgo
- favorites search engines: DUCKDUCKGO, google, wikipedia, youtube.

So, I thought that update script ran just when no more useful (after my own update). Switching back to 4.8 works (but obviously without last changes).
Comment 14 Dawit Alemayehu 2012-07-27 16:37:08 UTC
(In reply to comment #13)
> well.. it did not work here. Or better, it updated the variables just when I
> tried to set it again. Let me explain:
> My 4.8 situation:
> - search engine: google.
> - favorites search engines: google, duckduckgo, wikipedia, youtube
> After upgrade to 4.9
> - search engine: none
> - favorites search engines: none
> Then I tried to set them again, but instead of setting the old ones, I set
> duckduckgo as default and set google, wk and youtube as favorites.
> Actual situation
> - search engine: duckduckgo
> - favorites search engines: DUCKDUCKGO, google, wikipedia, youtube.
> 
> So, I thought that update script ran just when no more useful (after my own
> update). Switching back to 4.8 works (but obviously without last changes).

I have no idea why kconf_update did not work. When an update file gets installed it is supposed to automatically run and make all the necessary updates to existing config files. Can you check to see if your kuriikwsfilterrc file was actually updated ? If it was updated, the "update_info" key under section "$Version" should contain the entry "kuriikwsfilter.upd:pre-kde4.9".

If it did not, then kconf_update failed to run for some reason (at least on your system) and we need to figure out why.
Comment 15 Dawit Alemayehu 2012-07-28 02:51:27 UTC
(In reply to comment #14)
> (In reply to comment #13)
> > well.. it did not work here. Or better, it updated the variables just when I
> > tried to set it again. Let me explain:
> > My 4.8 situation:
> > - search engine: google.
> > - favorites search engines: google, duckduckgo, wikipedia, youtube
> > After upgrade to 4.9
> > - search engine: none
> > - favorites search engines: none
> > Then I tried to set them again, but instead of setting the old ones, I set
> > duckduckgo as default and set google, wk and youtube as favorites.
> > Actual situation
> > - search engine: duckduckgo
> > - favorites search engines: DUCKDUCKGO, google, wikipedia, youtube.
> > 
> > So, I thought that update script ran just when no more useful (after my own
> > update). Switching back to 4.8 works (but obviously without last changes).
> 
> I have no idea why kconf_update did not work. When an update file gets
> installed it is supposed to automatically run and make all the necessary
> updates to existing config files. Can you check to see if your
> kuriikwsfilterrc file was actually updated ? If it was updated, the
> "update_info" key under section "$Version" should contain the entry
> "kuriikwsfilter.upd:pre-kde4.9".
> 
> If it did not, then kconf_update failed to run for some reason (at least on
> your system) and we need to figure out why.


BTW, you can force kconf_update to run and update your kuriikwsfilterrc by issuing the following command:
/usr/lib/kde4/libexec/kconf_update --check kuriikwsfilter.upd

NOTE: kconf_update might be installed at a different location on your system.
Comment 16 Andrea Diamantini 2012-07-28 08:16:11 UTC
I confirm kconf_update did not run in my system. In fact, there was no $version section in the config file. Then I ran it manually and it appeared.