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
Branch
firefox_bookmarks_fix_extraction (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 26217
Build 26235: arc lint + arc unit
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 ↗(On Diff #81664)

Missing start of string anchor (^)

185 ↗(On Diff #81664)

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 ↗(On Diff #81664)

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 81720

View Options

runners/bookmarks/browsers/firefox.cpp

Loading...