Differential D29326

Bookmarks Runner: Fix extraction of firefox profile
ClosedPublic
Actions

Authored by alex on May 1 2020, 12:57 PM.

Details

Reviewers
ngraham
bruns
broulik
meven
Group Reviewers
Plasma
Commits
R120:a960c028c71d: Bookmarks Runner: Fix extraction of firefox profile
Summary

BUG: 418526

The profile group with the value Default=1 is not always the default profile, for example in the profiles.ini file from the bug report:

[Profile1]
Name=default
IsRelative=1
Path=dun5pnnn.default
Default=1

[Profile0]
Name=default-default
IsRelative=1
Path=xewbk2mp.default-default

[Install11457493C5A56847]
Default=xewbk2mp.default-default
Locked=1

In the text you can see that the profile with dun5pnnn.default has the value Default=1,
but it is not the default profile.
The correct default profile is in the [Install11457493C5A56847] group with the key Default.

Test Plan

If you create a new profile it will be extracted as the default profile
(but the old value might still be cached see D28619).

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
alex created this revision.May 1 2020, 12:57 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMay 1 2020, 12:57 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
alex requested review of this revision.May 1 2020, 12:57 PM
ngraham edited the summary of this revision. (Show Details)May 1 2020, 2:33 PM
bruns requested changes to this revision.May 1 2020, 7:23 PM

The summary needs some text which works without any images

The code needs some comment which tells how the selection logic works and why it is as it is.

runners/bookmarks/browsers/firefox.cpp
184–190

Missing start of string anchor (^)

185

Can this ever be > 1?

This revision now requires changes to proceed.May 1 2020, 7:23 PM
alex added inline comments.May 2 2020, 6:16 AM
runners/bookmarks/browsers/firefox.cpp
185

For normal use not but if you have multiple versions of Firefox with multiple profiles installed and want to launch a profile in a specific version.

alex edited the summary of this revision. (Show Details)May 2 2020, 6:23 AM
alex updated this revision to Diff 81720.May 2 2020, 6:44 AM

Anchor in regex, comment

meven added a subscriber: meven.May 2 2020, 7:19 AM

We should not have screenshots in git commit message as they are not durable.
(Only in test plan section is tolerable)
I think you can change your comment, to include the ini text instead, and add comments saying ; here marked default ; here real default

alex edited the summary of this revision. (Show Details)May 2 2020, 9:15 AM
alex marked 3 inline comments as done.May 2 2020, 7:22 PM
meven accepted this revision.May 3 2020, 10:38 AM
alex added a comment.May 17 2020, 11:51 AM

Friendly Ping @bruns or may I land this already ?
And should this land on Plasma/5.19?

alex added a comment.EditedMay 26 2020, 10:03 AM

I guess it should be fine, meven accepted this and all the changes from bruns were implemented.

This revision was not accepted when it landed; it landed in state Needs Review.May 27 2020, 3:09 PM
Closed by commit R120:a960c028c71d: Bookmarks Runner: Fix extraction of firefox profile (authored by alex). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 83164

View Options

runners/bookmarks/browsers/firefox.cpp

Loading...