Re: [Okular-devel] [PATCHES] Customizable annotation tools

2013-05-18 Thread Fabio D'Urso
Hi,

As I said in IRC, I'd like to include customizable annotation tools in KDE 
4.11, so I'm resurrecting this old thread.

Since the original post, I've created a configurable-review-tools branch and 
pushed almost(*) all patches there.
They have slightly been modified since the OP, but the overall structure is 
the same:
 - Tools can be configured from the configuration panel
 - Tools are still described through XML data
 - Configuration is saved in XML format in okularpartrc via kcfg

I've merged master again, therefore it's possible to see a big dump of all 
changes with:
  git diff origin/master origin/configurable-review-tools

A (imho) better way to see changes is:
  gitk origin/configurable-review-tools ^origin/master --no-merges
which gives some sort of changelog.

In the first patches, you can safely skip analysis of code that directly 
creates XML strings, as it is replaced by safer QDom methods in 
3dcaa585587984a0d34daa41915dfe4d4d995d75.

If you prefer a single big review request instead, please let me know.

* = What's still missing is support for a new "polyline" tool, which I had 
initially posted, but I've noticed it needs some corrections (my first 
implementation ended the line on double-click, but kolourpaint uses right-
click, and I'd like to mirror it for consistency).

Thanks,
Fabio

On Wednesday, August 08, 2012 12:19:03 AM Albert Astals Cid wrote:
> El Dimecres, 1 d'agost de 2012, a les 03:04:31, Fabio D'Urso va escriure:
> > On Wednesday, August 01, 2012 12:32:06 AM Albert Astals Cid wrote:
> > > Can we see screenshots of the other GUI changes you made for lazy
> > > people?
> > 
> > Annotation widget changes (in all images old is on the left):
> > - Do not show the font selection field in pop-up annotations (cmp-popup)
> > 
> >I guess it was a forgotten if, because the same widget is used for
> >inplace text annotations, where configuring the font makes sense.
> > 
> > - Configurable line width in InkAnnotations (cmp-ink)
> > - Configurable inner color in polygons (cmp-polygon)
> > 
> >Note that even tough straight lines and polylines use the same widget,
> >the inner color field is not shown in those annotations. In other
> >words,
> >their properties dialog were not changed.
> > 
> > - Text alignment in inplace text annotations (cmp-inplace)
> > 
> >This one can be done better (eg with icons) in future patches. I had
> >had
> > 
> > a quick look at koffice source and I remember that there were different
> > cases for LTR and RTL languages, that's why I went the easy path.
> > 
> > The Identity configuration page is replaced by an Annotations page, which
> > contains the configurable tool list. You can see it in config-panel-new.
> > Note that the privacy notice now appears as tooltip for text box.
> > 
> > In config-panel-new, you can also see the dialog to create and edit tools.
> > As I said in the previous post, it uses the same widgets used by the
> > annotation properties dialog.
> > The caption is "Edit annotation tool" if the user is editing an existing
> > tool (like in the picture), or "Create annotation tool" if the user is
> > creating a new tool.
> > If the user hasn't set a customized name, the Name text box shows the
> > default tool name in gray (placeholder text).
> > The combo lets you choose the tool type. Changing its value also changes
> > the type of properties that can be configured. Available choices are:
> > Pop-up Note, Inline Note, Freehand Line, Straight Line, Polygon,
> > Polyline, Text markup, Geometrical shape, Stamp.
> > 
> > In this dialog, I used the term "Freehand Line" instead of "Ink", and
> > "Straight Line", "Polygon" and "Polyline" instead of the generic "Line"
> > which is used in the annotation properties dialog, because they sound
> > clearer to me. If you agree, I'd make another patch to make the annotation
> > properties dialog show these names too.
> > 
> > > It might be good adding some contents to
> > > PageViewAnnotator::defaultToolName
> > > since e.g. "Underline" can be read as a verb or as a noun.
> > 
> > Yes, I see the issue. Currently we have Highlighter, but also Polygon,
> > Stamp, Underlining and Ellipse, that are all nouns. I'll review that part.
> > 
> > > What happens when there are lots of items in the toolbar, do you get a
> > > double line of icons?
> > 
> > Yes. I didn't change it and the existing code can do that (toolbar-wrap).
> > 
> > P.S.: I'd like to change the Polygon icon too, because it's not a polygon
> > right now.
> 
> Works for me (as in "I agree to everything you say")
> 
> Cheers,
>   Albert
> 
> > Fabio

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel


[Okular-devel] [okular] [Bug 319625] Bookmarks & annotations lost for renamed documents

2013-05-18 Thread Alan Aversa
https://bugs.kde.org/show_bug.cgi?id=319625

--- Comment #17 from Alan Aversa  ---
(In reply to comment #16)
> Sure, but why would Okular try to open /tmp/np658.pdf ?

Okular wouldn't by itself, but the user might make Okular do so by clicking on
a bookmark associated with the document's old path/filename.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel


Re: [Okular-devel] Review Request 109021: Font selector for TextDocumentGenerator

2013-05-18 Thread Commit Hook

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109021/
---

(Updated May 18, 2013, 2:03 p.m.)


Status
--

This change has been marked as submitted.


Review request for Okular, Albert Astals Cid and Eike Hein.


Description
---

Development history:
https://github.com/azat/okular/compare/master...font-selector-for-plain-text-formats

http://quickgit.kde.org/?p=clones%2Fokular%2Fazatkhuzhin%2Fokular.git&a=commitdiff&h=font-selector-for-plain-text-formats&hp=master
(but it has some delay, about a day or so while this clone will be updated, 
maybe because of mirrors?)

Link to thread from mailing list:
http://comments.gmane.org/gmane.comp.kde.devel.okular/13279


Diffs
-

  CMakeLists.txt 543e6df 
  conf/textdocumentsettings.ui PRE-CREATION 
  core/textdocumentgenerator.h dd75c5c 
  core/textdocumentgenerator.cpp f370ded 
  core/textdocumentgenerator_p.h 749d6f2 
  core/textdocumentsettings.h PRE-CREATION 
  core/textdocumentsettings.cpp PRE-CREATION 
  core/textdocumentsettings_p.h PRE-CREATION 
  generators/epub/CMakeLists.txt f076ed9 
  generators/epub/generator_epub.h cef2879 
  generators/epub/generator_epub.cpp 59bb2bf 
  generators/epub/libokularGenerator_epub.desktop 5c853a3 
  generators/fictionbook/generator_fb.h a898397 
  generators/fictionbook/generator_fb.cpp 2317083 
  generators/fictionbook/libokularGenerator_fb.desktop 099268c 
  generators/ooo/converter.cpp 1124e2a 
  generators/ooo/generator_ooo.h 3441c7a 
  generators/ooo/generator_ooo.cpp 793ee58 
  generators/ooo/libokularGenerator_ooo.desktop 328ae26 
  generators/poppler/annots.cpp 3694188 
  generators/txt/CMakeLists.txt 473ea12 
  generators/txt/generator_txt.h 5c15ec4 
  generators/txt/generator_txt.cpp 93ca4aa 
  generators/txt/libokularGenerator_txt.desktop 235e23d 

Diff: http://git.reviewboard.kde.org/r/109021/diff/


Testing
---

Tested manually


Thanks,

Azat Khuzhin

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel


Re: [Okular-devel] Review Request 109021: Font selector for TextDocumentGenerator

2013-05-18 Thread Commit Hook

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109021/#review32738
---


This review has been submitted with commit 
1fdb0a0a0621d8b8fa032cc7d613cb2178575845 by Albert Astals Cid on behalf of Azat 
Khuzhin to branch master.

- Commit Hook


On May 17, 2013, 12:07 a.m., Azat Khuzhin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109021/
> ---
> 
> (Updated May 17, 2013, 12:07 a.m.)
> 
> 
> Review request for Okular, Albert Astals Cid and Eike Hein.
> 
> 
> Description
> ---
> 
> Development history:
> https://github.com/azat/okular/compare/master...font-selector-for-plain-text-formats
> 
> http://quickgit.kde.org/?p=clones%2Fokular%2Fazatkhuzhin%2Fokular.git&a=commitdiff&h=font-selector-for-plain-text-formats&hp=master
> (but it has some delay, about a day or so while this clone will be updated, 
> maybe because of mirrors?)
> 
> Link to thread from mailing list:
> http://comments.gmane.org/gmane.comp.kde.devel.okular/13279
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 543e6df 
>   conf/textdocumentsettings.ui PRE-CREATION 
>   core/textdocumentgenerator.h dd75c5c 
>   core/textdocumentgenerator.cpp f370ded 
>   core/textdocumentgenerator_p.h 749d6f2 
>   core/textdocumentsettings.h PRE-CREATION 
>   core/textdocumentsettings.cpp PRE-CREATION 
>   core/textdocumentsettings_p.h PRE-CREATION 
>   generators/epub/CMakeLists.txt f076ed9 
>   generators/epub/generator_epub.h cef2879 
>   generators/epub/generator_epub.cpp 59bb2bf 
>   generators/epub/libokularGenerator_epub.desktop 5c853a3 
>   generators/fictionbook/generator_fb.h a898397 
>   generators/fictionbook/generator_fb.cpp 2317083 
>   generators/fictionbook/libokularGenerator_fb.desktop 099268c 
>   generators/ooo/converter.cpp 1124e2a 
>   generators/ooo/generator_ooo.h 3441c7a 
>   generators/ooo/generator_ooo.cpp 793ee58 
>   generators/ooo/libokularGenerator_ooo.desktop 328ae26 
>   generators/poppler/annots.cpp 3694188 
>   generators/txt/CMakeLists.txt 473ea12 
>   generators/txt/generator_txt.h 5c15ec4 
>   generators/txt/generator_txt.cpp 93ca4aa 
>   generators/txt/libokularGenerator_txt.desktop 235e23d 
> 
> Diff: http://git.reviewboard.kde.org/r/109021/diff/
> 
> 
> Testing
> ---
> 
> Tested manually
> 
> 
> Thanks,
> 
> Azat Khuzhin
> 
>

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel


[Okular-devel] [okular] [Bug 319426] Printing text-only PDF files results in empty pages

2013-05-18 Thread Robi
https://bugs.kde.org/show_bug.cgi?id=319426

--- Comment #14 from Robi  ---
(In reply to comment #13)
> Ah, forgot to read that exporting GS_FONTPATH you made gs work, yeah, that's
> sounding to me like a distro problem, you may want to check in some gentoo
> forum, since it works fine for me.
> 
> I'm going to close the bug now since the ps file we generate "works" and it
> seems you have some problem somewhere in the way from that ps file to the
> printer, maybe cups not finding your fonts or something.
> 
> If you find any proof that it is actually us doing something wrong, do not
> hesitate to reopen the bug.

Thanks for your support. 
I will try to ask the gentoo forum.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel


Re: [Okular-devel] Review Request 110480: Fix for removing setInplaceText() in r110391 (compilation fails)

2013-05-18 Thread Azat Khuzhin

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110480/
---

(Updated May 18, 2013, 11:52 a.m.)


Status
--

This change has been discarded.


Review request for Okular and Albert Astals Cid.


Description
---

Seems than poppler used it.
And compilation error occurs.


Diffs
-

  generators/poppler/annots.cpp 3694188 

Diff: http://git.reviewboard.kde.org/r/110480/diff/


Testing
---

Compiling is ok


Thanks,

Azat Khuzhin

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel


Re: [Okular-devel] Review Request 110480: Fix for removing setInplaceText() in r110391 (compilation fails)

2013-05-18 Thread Azat Khuzhin


> On May 18, 2013, 11:47 a.m., Fabio D'Urso wrote:
> > Hi, thank you for noticing, however this patch looks wrong to me: it's 
> > assigning txtann's contents variable to itself.
> > 
> > I remember why I added this special case, because Poppler < 0.20 sometimes 
> > stored garbage in the DOM node corresponding to inplaceText (but good data 
> > in contents).
> > This code copied the good value into inplaceText, so that Okular's UI code, 
> > which used to show the user what it read from inplaceText, showed the good 
> > value instead.
> > 
> > Now that Okular only has contents, this special handling is no longer 
> > needed, therefore that block can be removed at all. I'm fixing it myself.
> > Thank you again for testing poppler 0.18, which I had forgotten to check 
> > when I reviewed the other patch :)

You are welcome.
So I just close this issue.


- Azat


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110480/#review32720
---


On May 17, 2013, 12:13 a.m., Azat Khuzhin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110480/
> ---
> 
> (Updated May 17, 2013, 12:13 a.m.)
> 
> 
> Review request for Okular and Albert Astals Cid.
> 
> 
> Description
> ---
> 
> Seems than poppler used it.
> And compilation error occurs.
> 
> 
> Diffs
> -
> 
>   generators/poppler/annots.cpp 3694188 
> 
> Diff: http://git.reviewboard.kde.org/r/110480/diff/
> 
> 
> Testing
> ---
> 
> Compiling is ok
> 
> 
> Thanks,
> 
> Azat Khuzhin
> 
>

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel


Re: [Okular-devel] Review Request 110480: Fix for removing setInplaceText() in r110391 (compilation fails)

2013-05-18 Thread Fabio D'Urso

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110480/#review32720
---


Hi, thank you for noticing, however this patch looks wrong to me: it's 
assigning txtann's contents variable to itself.

I remember why I added this special case, because Poppler < 0.20 sometimes 
stored garbage in the DOM node corresponding to inplaceText (but good data in 
contents).
This code copied the good value into inplaceText, so that Okular's UI code, 
which used to show the user what it read from inplaceText, showed the good 
value instead.

Now that Okular only has contents, this special handling is no longer needed, 
therefore that block can be removed at all. I'm fixing it myself.
Thank you again for testing poppler 0.18, which I had forgotten to check when I 
reviewed the other patch :)

- Fabio D'Urso


On May 17, 2013, 12:13 a.m., Azat Khuzhin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110480/
> ---
> 
> (Updated May 17, 2013, 12:13 a.m.)
> 
> 
> Review request for Okular and Albert Astals Cid.
> 
> 
> Description
> ---
> 
> Seems than poppler used it.
> And compilation error occurs.
> 
> 
> Diffs
> -
> 
>   generators/poppler/annots.cpp 3694188 
> 
> Diff: http://git.reviewboard.kde.org/r/110480/diff/
> 
> 
> Testing
> ---
> 
> Compiling is ok
> 
> 
> Thanks,
> 
> Azat Khuzhin
> 
>

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel