Ticket #9112 (closed defect: fixed)

Opened 6 years ago

Last modified 3 years ago

Enable Browse to embed PDF files in itself

Reported by: sayamindu Owned by: erikos
Priority: high Milestone: 1.5-software-later
Component: browse-activity Version: not specified
Keywords: Cc: dsd, bernie, sascha_silbe
Action Needed: test in release Verified: no
Deployments affected: Blocked By:
Blocking:

Description

Browse should have a PDF viewer plugin so that users do not have to download a given PDF into the journal to read it. This will also function as a workaround for #8155

Attachments

embed_pdf_in_browse.patch (0.7 kB) - added by sayamindu 6 years ago.
Patch to implement the behaviour
Browse-101.xo (287.1 kB) - added by sayamindu 6 years ago.

Change History

Changed 6 years ago by sayamindu

Patch to implement the behaviour

follow-up: ↓ 7   Changed 6 years ago by sayamindu

The general idea is that we provide Uruguay with the attached bundle , while commit the patch in upstream Sugar. The patch adds the ability in Browse to add virtually any Mozilla/Firefox plugin (just drop the relevant files in the plugins folder in the Browse directory and spin a .xo bundle with that)

  Changed 6 years ago by sayamindu

  • next_action changed from never set to test in build

| TestCase |

* Download and install the attached Browse Bundle * Try to open any PDF file in ** the library (in the book section) ** on the web (eg: http://dev.laptop.org/~sayamindu/sample_pdfs) * Keep a file in the journal (click on the Journal icon on the extreme right of the toolbar of the embedded PDF viewer) ** Verify that the file gets into the journal and can be opened with Read

  Changed 6 years ago by sayamindu

One issue which remains is: how do we provide the source code for

a) mozplugger

b) m4

both of which are in the bundle for UY. Do I include the source for both in the bundle ?

Changed 6 years ago by sayamindu

follow-up: ↓ 6   Changed 6 years ago by cscott

Is m4 strictly necessary? Could its job be handled by sed instead? If not, we could probably add m4 to the build fairly easily, it's small.

Same thing for mozplugger? How big are the binaries vs how big is the compressed source?

  Changed 6 years ago by mstone

  • keywords staging-7:+ added
  • next_action changed from test in build to add to release

Sayamindu's testcase passes on staging-7 with the Browse-101.xo attached to this ticket. (I only tested with his sample PDFs, not with a PDF from the library.)

in reply to: ↑ 4   Changed 6 years ago by sayamindu

Replying to cscott:

Is m4 strictly necessary? Could its job be handled by sed instead? If not, we could probably add m4 to the build fairly easily, it's small.

It might be doable with sed, but I really do not want to touch mozplugger code right now.

Same thing for mozplugger? How big are the binaries vs how big is the compressed source?

The mozplugger SRPM is 77K.

in reply to: ↑ 1   Changed 5 years ago by dsd

Replying to sayamindu:

The general idea is that we provide Uruguay with the attached bundle , while commit the patch in upstream Sugar. The patch adds the ability in Browse to add virtually any Mozilla/Firefox plugin (just drop the relevant files in the plugins folder in the Browse directory and spin a .xo bundle with that)

Is that still the plan? If so, it looks like we have wrapped up this ticket for 8.2.1, and the next step is to get the code in a form acceptable for upstream sugar?

  Changed 5 years ago by dsd

  • cc dsd added

  Changed 5 years ago by sayamindu

  • next_action changed from add to release to finalize

The code is already in the process of getting into upstream. http://dev.sugarlabs.org/ticket/103

  Changed 5 years ago by sj

This is less general than the original request to allow specific programs to launch from Browse (by adding them to a whiteliste if necessary, for instance)... we'll want to support more than just PDF browsing -- for instance Read would best be able to launch and process any file format that it supports from a Browse link.

  Changed 4 years ago by Quozl

  • keywords staging-7:+ removed
  • next_action changed from finalize to review
  • milestone changed from 8.2.1 to 1.5-software-later

We've lost this feature in the changes from Browse-102 to Browse-108, the viewer binary is no longer present. Feedback from a deployment team suggests that lesson plans for 0.82 depend on the single click to read nature of collections of PDF files. XO-1.5 builds currently require four clicks.

  Changed 4 years ago by erikos

  • next_action changed from review to package

I will create a new bundle (based on 108) containing the mozplugger. As there has been a change in evince the sugar-pdf-viewer must change slightly, too (if hasattr(evince, 'evince_embed_init').

I tested the old binaries (http://dev.laptop.org/~sayamindu/pdf_embed/) working fine on the f11 builds, we should however maybe rebuild it based on the latest f11 package ( http://koji.fedoraproject.org/koji/buildinfo?buildID=112699 ).

follow-up: ↓ 16   Changed 4 years ago by erikos

A new .xo based on 108 is available at: http://dev.laptop.org/~erikos/bundles/0.84/Browse-108.xo

Please test.

Someone needs to help me make it available in a build then.

  Changed 4 years ago by mikus

Applied the new Browse-108 to os125. It shows the Africa map in the OLPC Library, as well as .pdf files accessed by manually entering the URL (i.e., 'file:///whatever.pdf'). [Or by first using Browse to view a page with links to .pdf files, then ("one click") clicking on one of those links.]

However, for the new Browse-108 to be able to show .pdf files from Journal, I had to add 'application/pdf;' to its activity.info file.

Even then, on os125 it was 'Read' rather than 'Browse' that was the default choice for displaying PDF files from Journal. I had to hover over the Journal entry's icon, click once in the palette on 'Resume with', then click again on 'Browse' -- that adds up to "two clicks".

  Changed 4 years ago by erikos

Mikus, thanks for testing.

Regarding the behavior with Read. Read is the default activity for reading pdfs. Was 8.2 different in that regard?

in reply to: ↑ 13   Changed 4 years ago by Quozl

Replying to erikos:

A new .xo based on 108 is available at: http://dev.laptop.org/~erikos/bundles/0.84/Browse-108.xo Please test.

Test passed on os126 with the above .xo loaded.

  Changed 4 years ago by cjb

  • priority changed from normal to high
  • next_action changed from package to test in release

test in os205

  Changed 4 years ago by Quozl

  • status changed from new to closed
  • resolution set to fixed

Tested in os205, with included Browse-108, in the default start page of the browser, clicked on books, books and texts, solar system ... and the PDF was displayed within Browse.

Also, clicking on a link to a PDF in a web page (apache simple file listing) results in correct display within Browse. Worked fine with file names containing spaces or not.

follow-up: ↓ 21   Changed 4 years ago by bernie

Can we send this patch to the list, so we can apply it to the latest version of Browse as well?

I think the current maintainers are lucian and silbe.

  Changed 4 years ago by bernie

  • cc bernie added

in reply to: ↑ 19   Changed 4 years ago by Quozl

Replying to bernie:

Can we send this patch to the list, so we can apply it to the latest version of Browse as well?

Yes, sure, you may. However, it will involve some difficulty; the current Browse from Sugar Labs is not 0.84 compatible. The solution we've used here has not been acceptable to reviewers in Sugar Labs. See Sugar Labs bug #103 for the current review comments.

  Changed 3 years ago by sascha_silbe

  • cc sascha_silbe added
Note: See TracTickets for help on using tickets.