Bug 118881 - Personas not found again (seems root cause is different than tdf#114731)
Summary: Personas not found again (seems root cause is different than tdf#114731)
Status: CLOSED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
6.0.6.1 rc
Hardware: x86-64 (AMD64) Linux (All)
: high major
Assignee: Muhammet Kara
URL:
Whiteboard: target:6.2.0 target:6.1.2 target:6.0.7
Keywords:
: 119239 119278 119327 119654 (view as bug list)
Depends on:
Blocks: Firefox-Themes
  Show dependency treegraph
 
Reported: 2018-07-22 13:20 UTC by Julien Nabet
Modified: 2018-09-14 17:53 UTC (History)
13 users (show)

See Also:
Crash report or crash signature:


Attachments
bt with debug symbols (4.74 KB, text/plain)
2018-07-23 09:37 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Julien Nabet 2018-07-22 13:20:16 UTC
Description:
On pc Debian testing package 6.0.6, I got no personas.
(idem with master sources updated today).

Steps to Reproduce:
1. Open Writer
2. Menu Options/Personalization
3. Click "Own Theme" then "Select Theme" button
=> a new window appears titled "Select Firefox Theme"
4. Click any button of "Categories" and wait a bit until "Searching, please wait..." message disappears.


Actual Results:
No theme retrieved

Expected Results:
Should retrieve some themes


Reproducible: Always


User Profile Reset: Yes



Additional Info:
Comment 1 Julien Nabet 2018-07-22 13:22:47 UTC
Confirmed with https://bugs.documentfoundation.org/show_bug.cgi?id=114731#c22
Comment 2 Julien Nabet 2018-07-23 09:31:06 UTC
Trying some debugging on gdb, I noticed several points:
1. when adding some traces, it seems block retrieved from urls doesn't contain "data-browsertheme" (see https://opengrok.libreoffice.org/xref/core/cui/source/options/personalization.cxx#577)

2. we expect """ for a quote but these same traces don't show any of them

3. textcolor attribute (see https://opengrok.libreoffice.org/xref/core/cui/source/options/personalization.cxx#590) may be null.
"textcolor":null

4. urls have some kind of encoding, don't know if it was always the case:
"previewURL":"https:\u002F\u002Faddons.cdn.mozilla.net\u002Fuser-media\u002Faddons\u002F420822\u002Fpreview_large.jpg?modified=8c8d0aab"
Comment 3 Julien Nabet 2018-07-23 09:36:40 UTC
UX-team: I've posted a message this morning on dev mailing list (see http://document-foundation-mail-archive.969070.n3.nabble.com/About-Persona-lightweight-themes-td4244320.html) but think you may be interested in this one.

Indeed, I wonder if this feature is really used.
Just my opinion but I think there's enough work with icon themes to add some extra work by maintaining this feature.
Comment 4 Julien Nabet 2018-07-23 09:37:54 UTC
Created attachment 143715 [details]
bt with debug symbols

bt showing where there's the pb with source URL.
Comment 5 Heiko Tietze 2018-07-23 09:57:39 UTC
needsUXEval needs CC ux-advice (will reply on the ML)
Comment 6 Buovjaga 2018-07-23 19:07:08 UTC
Just a guess, broken because of this: https://blog.mozilla.org/addons/2018/07/12/upcoming-changes-for-themes/ ?
Comment 7 Julien Nabet 2018-07-23 19:29:22 UTC
(In reply to Buovjaga from comment #6)
> Just a guess, broken because of this:
> https://blog.mozilla.org/addons/2018/07/12/upcoming-changes-for-themes/ ?

Good catch! It may indeed be related!
Comment 8 Peter Maunder 2018-07-29 13:39:44 UTC
Bug  59611 UI: Give personas an own entry in Tools Menu gives an option for users to develop test and use their own personas without needing online Firefox. I documented my offline use in attachment 127480 [details]. 

I hope any changes to the Personas/themes support will not stop this function from working.
Comment 9 Julien Nabet 2018-08-13 08:51:31 UTC
*** Bug 119239 has been marked as a duplicate of this bug. ***
Comment 10 Julien Nabet 2018-08-13 09:04:21 UTC
I've just noticed this sentence on the blog:
"The preview feature will be disabled starting today"

Shouldn't we remove preview part and just keep install/uninstall dialog?
Comment 11 Julien Nabet 2018-08-14 20:10:00 UTC
*** Bug 119278 has been marked as a duplicate of this bug. ***
Comment 12 Julien Nabet 2018-08-14 20:12:00 UTC
Let's increase importance of this one considering it's a regression and it's indeed used (see dups).
Comment 13 Julien Nabet 2018-08-17 09:58:43 UTC
*** Bug 119327 has been marked as a duplicate of this bug. ***
Comment 14 Julien Nabet 2018-09-03 08:04:23 UTC
*** Bug 119654 has been marked as a duplicate of this bug. ***
Comment 15 Thomas Lendo 2018-09-04 19:59:19 UTC
I like this feature very well. It's something MSO hasn't and it's similar to Firefox browser which also many know. If it's possible in any way, please don't remove this unique office suite feature.
Comment 16 Heiko Tietze 2018-09-05 18:48:48 UTC
From the UX POV we have some options:
a) remove this feature completely
b) don't access images via network but ship a few and make the feature an extension
c) revamp the code to adopt the new API

Without any user data I cannot say how often personas are used. So a) is not a good solution. And c) sounds like a lot of work, similar to b) but we would gain full control over the persona configuration. Plus, this approach strengthens the extensions and allows the design team to make things fancy. My take is b).
Comment 17 Julien Nabet 2018-09-05 18:52:02 UTC
(In reply to Heiko Tietze from comment #16)
>...
> c) revamp the code to adopt the new API
Where can the new API be found?
Comment 18 Heiko Tietze 2018-09-05 18:53:36 UTC
(In reply to Julien Nabet from comment #17)
> Where can the new API be found?

needsDevAdvice?
Comment 19 Julien Nabet 2018-09-05 19:00:53 UTC
(In reply to Heiko Tietze from comment #18)
> (In reply to Julien Nabet from comment #17)
> > Where can the new API be found?
> 
> needsDevAdvice?

Indeed because looking at the comments in https://blog.mozilla.org/addons/2018/07/12/upcoming-changes-for-themes/, nothing is indicated about a new API.

IMHO, if Mozilla doesn't provide any API quickly (in some weeks and not some years), solution a) (remove the feature) should be considered seriously
Comment 20 Buovjaga 2018-09-06 07:11:05 UTC
(In reply to Julien Nabet from comment #19)
> (In reply to Heiko Tietze from comment #18)
> > (In reply to Julien Nabet from comment #17)
> > > Where can the new API be found?
> > 
> > needsDevAdvice?
> 
> Indeed because looking at the comments in
> https://blog.mozilla.org/addons/2018/07/12/upcoming-changes-for-themes/,
> nothing is indicated about a new API.
> 
> IMHO, if Mozilla doesn't provide any API quickly (in some weeks and not some
> years), solution a) (remove the feature) should be considered seriously

This older article https://blog.mozilla.org/addons/2018/03/08/theme-api-update/ says "Static themes, which only contain a manifest and image resources, will be supported on addons.mozilla.org (AMO) early this year. These will replace LightWeight Themes in a backward and forward compatible way."

Static themes are explained here: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/Themes/Theme_concepts#Static_themes

It seems support for them has been fully implemented less than a month ago: https://github.com/mozilla/addons-frontend/projects/6 but maybe the Addons site is still not ready to accept them.

Yet, I think we should remember the biggest issue: the previews are gone! Nothing I've read indicated they will be coming back. I think this is a strong point in favour of removing the feature completely.
Comment 21 Buovjaga 2018-09-06 07:14:57 UTC
(In reply to Buovjaga from comment #20)
> Yet, I think we should remember the biggest issue: the previews are gone!
> Nothing I've read indicated they will be coming back. I think this is a
> strong point in favour of removing the feature completely.

Just to make this crystal clear:
https://github.com/mozilla/addons-frontend/issues/3056
"Since future web-ext based lightweight themes don't allow preview and install and uninstall is quick and easy I'd like to propose we remove the theme preview feature everywhere."
Comment 22 Julien Nabet 2018-09-06 07:38:39 UTC
Michael: any thoughts about followings to give about this one? (see last comments)
Comment 23 Buovjaga 2018-09-06 07:41:39 UTC
Stephan's thoughts: https://lists.freedesktop.org/archives/libreoffice/2018-September/080894.html
"In general, be careful when including new kinds of data content in extensions.  That creates new interfaces between LO and extensions, and those interfaces need to be kept compatible, which is a maintenance burden."
Comment 25 Muhammet Kara 2018-09-13 10:31:22 UTC
What I have by taking a quick look:

- There is no problem with the API part (The API we use is indeed deprecated but still functional, the API change of mozilla is something different (not related to our use case) AFAIK)
- Seems like there has been some change on the design of the individual persona/theme pages on AMO (addons.mozilla.org) which causes our parser methods to fail

I can quickly cook up a patch to make our Personas search feature operational again, but we better think about a way to improve this feature overall, and make it future-proof. Mozilla might make design changes anytime again which breaks things on our side.

Random thoughts:
- Select a small set of personas (maybe 10 or 20?), and ship them with LO
- Include an index file for a larger set of Personas, which might be updated by a script time to time (maybe getting most-downloaded 100 (or 1000?) Personas), and perform the searches on this set of Personas.
Comment 26 Michael Meeks 2018-09-13 10:56:06 UTC
For my part - I think shipping a small set of default personas with the product would be really useful. Microsoft ship (I think) three ;-) and make it easy to select one when you first start / upgrade. So while we shipped the feature first - their users actually use it ;-)
Comment 27 Muhammet Kara 2018-09-13 15:52:45 UTC
Patch on gerrit makes it functional again: https://gerrit.libreoffice.org/#/c/60432/
Comment 28 Commit Notification 2018-09-13 19:36:39 UTC
Muhammet Kara committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=e98ac43ec42ff398ad489d6719960d595f0327be

tdf#118881: Fix HTML parsing for personas

It will be available in 6.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 29 Commit Notification 2018-09-13 23:03:46 UTC
Muhammet Kara committed a patch related to this issue.
It has been pushed to "libreoffice-6-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=2f52a8e0f1098a51631434129707cfb0b60fecb3&h=libreoffice-6-1

tdf#118881: Fix HTML parsing for personas

It will be available in 6.1.2.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 30 Muhammet Kara 2018-09-14 06:32:29 UTC
Seems resolved.
Comment 31 Xisco Faulí 2018-09-14 08:57:23 UTC
Cherry-picked to 6-0 as well: https://gerrit.libreoffice.org/#/c/60481/
Comment 32 Xisco Faulí 2018-09-14 08:58:05 UTC
removing keywords as this wasn't a regression caused by us
Comment 33 Xisco Faulí 2018-09-14 09:14:03 UTC
Verified in

Version: 6.2.0.0.alpha0+
Build ID: 4e5f89d2d3511b6421b388ecaba2f61ada14d084
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: threaded

@Muhammet Kara, thanks for fixing this!!
Comment 34 Commit Notification 2018-09-14 11:05:47 UTC
Muhammet Kara committed a patch related to this issue.
It has been pushed to "libreoffice-6-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=6a113caf17974eb07a9f06ef09b1b1737d4d07bc&h=libreoffice-6-0

tdf#118881: Fix HTML parsing for personas

It will be available in 6.0.7.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 35 Julien Nabet 2018-09-14 17:53:51 UTC
On pc Debian x86-64 with master sources updated today, I don't reproduce this anymore.

Thank you Muhammet!

Let's close this one then.