Skip to content
This repository has been archived by the owner on Sep 16, 2019. It is now read-only.

Adds option to override the theme's icons through monochrome icons and fixes string encoding issues #25

Closed
wants to merge 3 commits into from

Conversation

kostrykin
Copy link
Contributor

@kostrykin kostrykin commented Aug 2, 2016

I would appreciate it very much if you could push these changes to Launchpad.
#1. Fixes #23

bildschirmfoto vom 2016-08-02 13 51 02

bildschirmfoto vom 2016-08-02 13 51 44
#2. Fixes #24

It has been observerd that Zotero encodes its strings sometimes twice recursively. The modifications of this commit try to cope with that by decoding each string up to three times.

Probably this also fixes #1?

It has been observerd that Zotero encodes its strings *sometimes* twice recursively. The modifications of this commit try to cope with that by decoding each string up to three times.
@kostrykin kostrykin changed the title Adds option to override the theme's icons through monochrome icons Adds option to override the theme's icons through monochrome icons and fixes string encoding issues Aug 2, 2016
@smathot
Copy link
Owner

smathot commented Aug 2, 2016

Hi,

Thanks for your input. I'll take a look at it soon. The encoding fix looks pretty particularly interesting--that has defeated me for a long time!

Cheers,
Sebastiaan

Fixes the project version from 2.2.1 to 2.1.1 because it makes more sense.
Copy link
Owner

@smathot smathot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again for this. The encoding issue is not fixed yet, see my comment below. What do you think?

@@ -26,6 +26,33 @@
import time
from libzotero.zotero_item import zoteroItem as zotero_item

def decode_zotero_str(raw, max_iters=3):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me, this completely breaks the PDF attachments. This appears to be due to my own weird way of trying to fix the encoding. If I comment the line below out (from LibZotero.update()), it works again on my system.

item_attachment = item_attachment.encode('latin-1').decode('utf-8')

However, the fact that a patch that for you seems to fix things, breaks things for me makes me a bit worried. On what operating system do you work? It's not unthinkable that the encoding differs between operating systems.

Copy link
Contributor Author

@kostrykin kostrykin Oct 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on Linux. I have that fork running on 3 different systems,

  • two of which are Ubuntu 14.04 based Elementary OS Freya
  • and one is Ubuntu 16.04 based Elementary OS Loki.

What operating system are you on?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What operating system are you on?

I'm on Ubuntu 16.04.

It has been observerd that Zotero encodes its strings sometimes twice recursively. The modifications of this commit try to cope with that by decoding each string up to three times.

What makes you think this? Is this a conclusion based on the fact the decoding multiple times resolves the encoding issues, at least in some cases? Or is it a known bug in the Zotero code?

I'll think about this. The combination of my odd hack (clearly dating from before understood how encoding really works) and your fix is nonsensical; but it may be that your fix on its own is correct. But I'd like to do some more tests before I push another fix that actually doesn't fix things.

(As you may have noticed, I don't have that much time to work Qnotero anymore. But I'll get to it!)

Copy link
Contributor Author

@kostrykin kostrykin Oct 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds very reasonable!

To answer your question: My fix was based on the observation that Zotero encoded 'Öf' (a two-letters string, where the first letter is "Ö", that is a non-ASCII) as the five-bytes sequence \xc3\x83\xc2\x96\x66. This is strange, because UTF-8 encodes ASCII letters as single bytes and non-ASCII characters as a sequence of two bytes. Empirically, I ruled out the possibility of that being a UTF-16 encoding. So why are two letters encoded as five bytes?

In [14]: '\xc3\x83\xc2\x96\x66'.decode('utf-8')
Out[14]: u'\xc3\x96f'

Turns out, that \xc3\x83\xc2\x96\x66 is the UTF-8 encoding for the three-characters-string \xC3\x96f (note the f at the end). The first two bytes \xC3\x96 are in turn the UTF-8 representation of Ö. So I suspect that this is an odd behaviour from Zotero.


Update: In an earlier version of this, I miscounted the number of bytes in the exemplary sequence and wrote that it were 4, although it are 5.

@@ -41,7 +41,7 @@ def __init__(self, qnotero):
self.setWindowProperties()
self.setScrollBars()

def icon(self, iconName):
def icon(self, iconName, overrideIconExt=None):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other themes should be updated as well. Right now this breaks the chameleon theme.

@firstuanl
Copy link

i use ubuntu16.04, like
For me, this completely breaks the PDF attachments. This appears to be due to my own weird way of trying to fix the encoding. If I comment the line below out (from LibZotero.update()), it works again on my system.
now, comment the line below out (from LibZotero.update()), open link attachment is ok

@kostrykin kostrykin closed this by deleting the head repository Nov 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants