D27722: [resources/maildir] Don't save "file:" schema to the config

[Igor Poboiko]

poboiko created this revision.
poboiko added reviewers: dvratil, mlaurent.
Herald added a project: KDE PIM.
poboiko requested review of this revision.

REVISION SUMMARY
  `ConfigWidget` uses `KConfig` underneath, and utilizes `KUrlRequester` custom
  widget. The `USER` property of this widget (which is used by `KConfig`) is of
  type `QUrl`, and thus when dialog is accepted, the `path` config property
  gets overriden with `QUrl::toString()` value, which prepends `file:` schema
  (this is basically because `KCoreConfigSkeleton::ItemPath` is inherited from
  `ItemString`, and when someone calls `ItemString::setProperty`, it gets
  casted as `QVariant::toString`).
  
  Inside the `ConfigWidget::save` the code calls `setPath` method on
  `url.toLocalFile`, which drops the scheme. Because of that, the `pathItem`
  and `path` property of `mSettings` have different values, first has schema
  and the second hasn't. Eventually, the value stored by `pathItem` wins, and
  `mSettings->path()` returns URL with schema. However, `Maildir` doesn't expect
  it and misinterprets it as the relative path to current WORKDIR (which is home
  directory), thus creating `/home/user/file:/home/user/...` file structure.
  
  The proposed solution is to simply call `mSettings->save()`, which overrides
  `pathItem` value and drops schema from it.
  
  It also fixes the `AkoNotes` resource, which uses the same `ConfigWidget`.
  Funny enough, `Contacts` resource, which is somewhat similar, is not affected
  as it has the same `Settings->save()` call.
  
  Alternative approaches include:
  
  1. Teach `Maildir` to drop the schema (if it's there).
  2. Teach `KCoreConfigSkeleton::ItemPath` to work with `QUrl` and don't append
  
  schema (it makes sense, because `ItemPath` corresponds to local file. Although
  it's not documented that it shouldn't have schema, it seems from the tests that
  it was the original intent). This could save the headache of having such issue
  in the future, but it could mess up with other programs in funny ways (as
  currently `file:` sometimes gets prepended, and some code might implicitly rely
  on it)
  
  Additional note:
  There are `ui.kcfg_Path->url().isLocalFile()` checks around, which doesn't make
  sense to me, as `KUrlRequester` is used for local files and it seems like it
  always returns `QUrl` pointing to local file (i.e. have the `file:` schema).
  
  BUG: 408354
  BUG: 411269
  BUG: 413588

TEST PLAN
  1. Open `akonadiconsole -> Local Folders` properties, change the folder, save
  2. `cat ~/.config/akonadi_maildir_resource_0rc`. `file:` schema gets prepended
  
  2.1) `akonadictl restart`. `file:` folder gets created inside homedir
  
  3. Apply patch, repeat (1)-(2.1). `file:` schema is dropped.

REPOSITORY
  R44 KDE PIM Runtime

BRANCH
  scheme-bug (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D27722

AFFECTED FILES
  resources/maildir/configwidget.cpp

To: poboiko, dvratil, mlaurent
Cc: kde-pim, fbampaloukas, dcaliste, dvasin, rodsevich, winterz, vkrause, mlaurent, knauss, dvratil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20200228/987ae237/attachment.html>