Fixes the crash for me, thanks!

Please use FEATURE: instead of BUG: in the commit message.

Looks good to me now, landing it.

@cfeck What if we use a KSqueezedTextLabel in KMessageWidget instead? That would mostly fix also the 'aaaaaaaaaaaa' case without requiring new API in QLabel.

• Added warnings

• Use untraslated string as fallback

You're welcome :)

Thanks. Do you have commit access?

In D18562#409491, @gepardo wrote:

Please remove the QEXPECT_FAIL at line 498 of cli7ztest.cpp, then this is good to go.

I don't have such line. Should I merge my branch with master?

Thanks, looks good to me now.

Thanks, that fixed the tests.

Failing test case added with 95bf2244e5b4c9fe7901fd50dd662dd514439407 (thanks for the test archive!)

Sorry but this patch breaks the following tests:

+1 otherwise.
Would be nice if we manage to not keep the http link in the URL navigator, but that's probably an unrelated issue.

Wait, do we need to add this workaround to every popup menu around? What about 3d-party apps?

LGTM

I'll review it soon. I want to add a test case with your test archive first; feel free to beat me to it :)

Addressed comments

Also, can you please update the Summary and the Test Plan? We should remove mentions of the option (which we dropped) and we should use FEATURE: instead of BUG:.

I'm not sure if I'm testing this patch the wrong way, but it doesn't seem to work here. What I did:

Can you please rebase the patch? There is new code in dolphinmainwindowtest which needs to be adapted.

Note: this will break with older clazy versions. I'm not sure how to prevent that.
One could argue that a developer interested in clazy should probably run the latest version anyway...

Thanks!

I'll push in a week unless there are objections.

I'm also a bit puzzled: are we removing a feature in order to show an error message instead?

I prefer your previous approach: an "observer" class is not supposed to have public slots, it should react on its own when stuff happens.

@anthonyfieroni Next time please add the summary of the revision to the actual commit message. You may want to setup arc to do that.

• Rebased

Ping

In D17635#396703, @ngraham wrote:

@elvisangelaccio would you be okay with changing the behavior to always focus the view upon opening a new tab as @emateli requests? I'm pretty much fine with anything, but I'd rather we decide on something so this poor patch can come to the end of its end its identity crisis. :-)

Thanks. Can you share a test archive that works with this patch but doesn't without?

LGTM now.

• Added set_package_properties to explain the optional feature

Note: I have no clue what KActivities::ResourceInstance actually does, but it doesn't seem to break anything either.

Sorry but I can't reproduce the bug you put in the commit message.

Thanks!

@arrowd I see you landed D18099, so this can be discarded, right?

In D17882#392432, @rizzitello wrote:

What should I be using as a base for this code? Its currently based upon the Applications/18.12 branch

For the record I still think that we should add another button rather than a new option. But anyway I think we discussed this enough, let's go with the option.

What else is missing to fix #402665?

+1

Well ok, you seem confident enough :)

In D17938#386999, @arrowd wrote:

In D17938#386985, @elvisangelaccio wrote:

I don't know, "cursorRow" and "cursorColumn" are pretty editor-specific. I'm not sure they make sense in the KRun API.

I'd say they are both filetype-specific and service-specific. But I haven't found any other approach.

I don't know, "cursorRow" and "cursorColumn" are pretty editor-specific. I'm not sure they make sense in the KRun API.

In D17617#384214, @ngraham wrote:

I don't have access to a webdav server to test with. Do you know of a public one?

Much better now, but I noticed that with webdavs:// the errore message does not show the name of the new folder:

Alternative fix: D17881