Ticket #3455 (closed defect: fixed)

Opened 13 months ago

Last modified 5 months ago

Browse: show link palette

Reported by: manuq Owned by: humitos
Priority: High Milestone: 0.98
Component: Browse Version: Git as of bugdate
Severity: Major Keywords: 13.1.0, r+, olpc-test-pending
Cc: erikos, godiard, humitos, manuq Distribution/OS: OLPC
Bug Status: Assigned

Description

Replace the default WebKitGTK menu for our custom palette.

For links the options were: follow link, follow link in new tab, keep link, copy link, start download.

And for images were: keep image, copy image.

Attachments

first_palettes.patch Download (13.4 KB) - added by erikos 13 months ago.
First patch towards palette support for link and images
0001-Show-Sugar-s-palette-when-right-click-is-pressed-SL-.patch Download (19.1 KB) - added by humitos 8 months ago.
0001-Show-Sugar-s-palette-when-right-click-is-pressed-SL-.2.patch Download (19.1 KB) - added by humitos 8 months ago.
v2 - "Keep link" implemented (and Simon added)
0001-Show-Sugar-s-palette-when-right-click-is-pressed-SL-.3.patch Download (19.6 KB) - added by humitos 8 months ago.
v3 - close the palette after clicking on the MenuItem
0001-Use-PaletteMenuItem-instead-local-SugarMenuItem-SL-3.patch Download (9.6 KB) - added by humitos 8 months ago.
0001-Use-PaletteMenuItem-instead-local-SugarMenuItem-SL-3.2.patch Download (9.7 KB) - added by humitos 6 months ago.
Last patch rebased

Change History

  Changed 13 months ago by manuq

  • status changed from new to assigned
  • component changed from untriaged to Browse
  • owner set to manuq
  • version changed from Unspecified to Git as of bugdate
  • milestone changed from Unspecified by Release Team to 0.96
  • keywords 12.1.0 added

  Changed 13 months ago by manuq

  • cc erikos added

  Changed 13 months ago by manuq

  • priority changed from Unspecified by Maintainer to High

  Changed 13 months ago by manuq

A related bug is #1222, we need to wrap the images menu that WebKit provides when right-clicking on them.

  Changed 13 months ago by manuq

Another related old bug: #946 .

Changed 13 months ago by erikos

First patch towards palette support for link and images

  Changed 13 months ago by erikos

  • distribution changed from Unspecified to OLPC
  • severity changed from Unspecified to Major
  • status_field changed from Unconfirmed to Assigned

The patch above does display the palettes for images and links. The link options "Following a link" and "Following it in a new tab" do work. The options for copying a link and an image to the clipboard do NOT work and downloading an image or a clipboard neither. The behavior is different in the sense that the palette will not popdown when the mouse moves away from the invoker widget (link or image), which has worked before. The palette needs to be dismissed by clicking on the underlying widget or it will go as well away when you move out of the palette itself.

* Copying to the clipboard is broken because 'gtk_clipboard_set_with_data' is not introspectable, see  http://www.daa.com.au/pipermail/pygtk/2011-July/019940.html.

* The downloading part needs to be simply implemented

* "the popdown of the palette when leaving the invoker widget" is a hard one, because a mouse-out event of an element is currently not supported in webkitgtk, see  https://lists.webkit.org/pipermail/webkit-gtk/2012-January/000908.html

  Changed 13 months ago by manuq

Great advance! Some feedback:

  • The title of the link is missing at the top of the palette.
  • Some URL links do not display at the top of the palette, like the "Log in" link at the top of the SugarLabs wiki.
  • The order is wrong, the right order is in the description of this ticket.

  Changed 12 months ago by humitos

  • keywords 12.1.0, patch added; 12.1.0 removed

  Changed 9 months ago by humitos

  • cc godiard, humitos added

The problem is still visible on build 14

follow-up: ↓ 13   Changed 8 months ago by humitos

What should happen when the user click over the text itself? I mean, no link, no image, just text... and what should happen if that text is selected?

Changed 8 months ago by humitos

  Changed 8 months ago by humitos

I uploaded a new patch with some improvements to eriko's patch. Please, take a look at it and let me know if there are something that I can do to solve the issues that it has.

  Changed 8 months ago by humitos

I couldn't find what is the meaning of the Menu Item: "Keep link".

Changed 8 months ago by humitos

v2 - "Keep link" implemented (and Simon added)

in reply to: ↑ 10   Changed 8 months ago by manuq

Replying to humitos:

What should happen when the user click over the text itself? I mean, no link, no image, just text... and what should happen if that text is selected?

This is common sense, you don't need to handle that.

  Changed 8 months ago by manuq

Great, v2 gets close to a solution. The problem I see is that the palette should be dismissed after you click in one item.

  Changed 8 months ago by humitos

  • owner changed from manuq to humitos
  • status changed from assigned to accepted

  Changed 8 months ago by manuq

  • keywords 13.1.0, added; 12.1.0, removed
  • milestone changed from 0.96 to 0.98

Changed 8 months ago by humitos

v3 - close the palette after clicking on the MenuItem

follow-ups: ↓ 18 ↓ 19   Changed 8 months ago by manuq

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

Great, pushed as c3b6dd91 . Note that we still have unsolved the popup when the mouse is hover. This patch does the popup only for the right click. The popup when hover should be recovered in the future.

in reply to: ↑ 17   Changed 8 months ago by humitos

Replying to manuq:

Great, pushed as c3b6dd91 . Note that we still have unsolved the popup when the mouse is hover. This patch does the popup only for the right click. The popup when hover should be recovered in the future.

This is followed here: #3967

in reply to: ↑ 17 ; follow-up: ↓ 20   Changed 8 months ago by erikos

Replying to manuq:

Great, pushed as c3b6dd91 . Note that we still have unsolved the popup when the mouse is hover. This patch does the popup only for the right click. The popup when hover should be recovered in the future.

This commit needs cleanup. There are debug logs in it, uncommented code, and it uses a local sugarmenuitem and not the one from the toolkit.

in reply to: ↑ 19   Changed 8 months ago by manuq

  • status changed from closed to reopened
  • resolution fixed deleted

Replying to erikos:

Replying to manuq:

Great, pushed as c3b6dd91 . Note that we still have unsolved the popup when the mouse is hover. This patch does the popup only for the right click. The popup when hover should be recovered in the future.


This commit needs cleanup. There are debug logs in it, uncommented code, and it uses a local sugarmenuitem and not the one from the toolkit.

Oh true. Reopening to fix it.

Changed 8 months ago by humitos

  Changed 8 months ago by erikos

  • keywords p? added; patch removed

  Changed 8 months ago by humitos

  • keywords r? added; p? removed

  Changed 7 months ago by manuq

Copy Image functionallity fixed by Gonzalo, dd7f9521 .

  Changed 6 months ago by erikos

  • cc manuq added
  • keywords r- added; r? removed

Humitos, can you rebase the above patch, it does not apply anymore. And please check if we have any layout regressions as we adjusted the PaleteMenuItem code recently.

Changed 6 months ago by humitos

Last patch rebased

  Changed 6 months ago by humitos

  • keywords r? added; r- removed

erikos, I've just attached the new version of this patch that applies in master.

  Changed 6 months ago by erikos

  • keywords r+, olpc-test-pending added; r? removed
  • status changed from reopened to closed
  • resolution set to fixed

Pushed this one. To test please verify that the copy link, follow link etc Palette is working.

  Changed 5 months ago by greenfeld

I am getting slightly confused testing this one.

A lot of times when I try to keep a link, open a picture link it in a new tab, etc., Sugar as a whole crashes and gets restarted in OLPC 13.1.0 os20/Browse-148.

I have not tracked this down yet, and sometimes going back to a page/link which previously caused a crash does not cause it a second time, and sometimes going back a second time causes a crash when the first time to do something with a resource does not.

Seen on XO-1.75 & XO-4.

  Changed 5 months ago by dsd

Those crashes are probably gone in the latest sugar releases, included in build 21.

Note: See TracTickets for help on using tickets.