Ticket #3066 (closed defect: fixed)

Opened 21 months ago

Last modified 6 weeks ago

BrowseV124 regressions

Reported by: tonyforster Owned by: erikos
Priority: Normal Milestone: 0.94
Component: Browse Version: Unspecified
Severity: Unspecified Keywords: 11.3.0
Cc: manuq Distribution/OS: Unspecified
Bug Status: Unconfirmed

Description

V124 does not start on Sugar 0.88 (OS373pyg)
File "/home/olpc/Activities/Browse.activity/webtoolbar.py", line 263, in init

self.entry.set_icon_from_name(gtk.ENTRY_ICON_SECONDARY,

AttributeError: 'module' object has no attribute 'ENTRY_ICON_SECONDARY'

Sugar 0.93 (1.75 OS41)
No right click menu
ctrl C does not copy address bar URL

(Noted previously Closing 1 tab exits Activity)

Change History

  Changed 21 months ago by erikos 

File "/home/olpc/Activities/Browse.activity/webtoolbar.py", line 263, in init

    self.entry.set_icon_from_name(gtk.ENTRY_ICON_SECONDARY,

AttributeError: 'module' object has no attribute 'ENTRY_ICON_SECONDARY'

This is known, the activities page should note that 124 is only compatible with 0.94.

Sugar 0.93 (1.75 OS41)
No right click menu

Can you elaborate here?

ctrl C does not copy address bar URL

Do you have steps to reproduce? It seems to work here.

  Changed 21 months ago by tonyforster

Right click works for the first opened tab but not for successive tabs

ctrl C : go to any page, select the url in the address bar, Ctrl C, nothing happens
!00% reproducable

  Changed 21 months ago by tonyforster

V125

Sugar 0.88 OS373pyg
Now starts OK
Address bar is not updated as you follow links
No history in the back button and grayed out
ctrl C address bar OK
Right mouse menu mostly OK

Sugar 0.93 OS41
Cannot ctrl-C from the address bar
Right mouse menu, mostly OK but maybe getting some erratic results, for example OLPC Library has links 'Release Notes' and 'Developer Key' with no right menu. Googling GGG gives Google Finance Graco Inc as top hit, this link has no right menu.

'Release Notes' and 'Developer Key' look like legitimate links except they include a font color, maybe thats why

  Changed 21 months ago by tonyforster

btw probably not relevant to ctrl-C but the 1.75 is a mechanical keyboard

follow-up: ↓ 6   Changed 21 months ago by erikos

  • milestone changed from Unspecified by Release Team to 0.94

Thanks Tony for your thorough testing!

The issue with the 'colored' link is a very nice catch:

<html>
<a href="http://laptop.org">olpc</a> 
<br><br>
<a href="http://laptop.org"><font color=blue>olpc-color</font></a> 
</html>

It is true that the 'colored-link' does not display the palette. In the invoker code we only do  check for the node enclosing the text. We could use  parentNode to solve the case above. But if it gets more nested we have the same issue. On the other hand I guess nested links are rare, because people define the color either in css or in the tag itself <a href="/" style="color:green">. Are there any other known non-working links?

in reply to: ↑ 5   Changed 21 months ago by erikos

Replying to erikos:

Thanks Tony for your thorough testing!

The issue with the 'colored' link is a very nice catch:

{{{
<html>
<a href=" http://laptop.org">olpc</a>
<br><br>
<a href=" http://laptop.org"><font color=blue>olpc-color</font></a>
</html>
}}}

It is true that the 'colored-link' does not display the palette. In the invoker code we only do  check for the node enclosing the text. We could use  parentNode to solve the case above. But if it gets more nested we have the same issue. On the other hand I guess nested links are rare, because people define the color either in css or in the tag itself <a href="/" style="color:green">. Are there any other known non-working links?

Ok, I have found some more. The google menu (images, maps...) which uses <span> for example and yes the example you gave uses as well one layer of nesting.

follow-up: ↓ 15   Changed 21 months ago by erikos 

diff --git a/palettes.py b/palettes.py
index 9fbc370..a275f37 100644
--- a/palettes.py
+++ b/palettes.py
@@ -77,8 +77,10 @@ class ContentInvoker(Invoker):
 
         target = event.target
 
-        if target.tagName.lower() == 'a':
+        if not (target.tagName.lower() == 'a' or target.tagName.lower() == 'img'):
+            target = target.parentNode
 
+        if target.tagName.lower() == 'a':
             if target.firstChild:
                 title = target.firstChild.nodeValue
             else:

The above would solve the issues above.

I came across another issue that is in the same code path. Often an image and a link are nested. For example, see the picture of Leonardo on  http://de.wikipedia.org/wiki/Da_Vinci and right click on it in Firefox. You will have the options to follow the link (new tab new window or copy it) or save the image. In Browse you can only save the image when using the palette.

  Changed 21 months ago by erikos

Btw, if we want to have tabs in Browse fully integrated we need to have an option in the link-palette for opening a link in a new Tab.

  Changed 21 months ago by tonyforster

another example is my webmail which nests b /b inside a link
(I changed triangular bracket because I am not sure what would happen here)

(a href="www.thing")(b)[Sugar-devel] [ANNOUNCE] New Dextrose-3 development build: Alpha-1(/b)(/a>)

follow-up: ↓ 11   Changed 21 months ago by erikos

About the issue of not being able to copy from the entry field: actually this never worked :) The code does only copy from the canvas. Selecting text there and copying it does work fine.

in reply to: ↑ 10   Changed 21 months ago by tonyforster

Replying to erikos:

About the issue of not being able to copy from the entry field: actually this never worked :) The code does only copy from the canvas. Selecting text there and copying it does work fine.

OS868au Sugar 0.84 Browse122 allows copy from the address bar

  Changed 21 months ago by erikos

  • keywords 11.3.0 added
  • owner changed from lucian to erikos
  • status changed from new to assigned

Ok, here the state of art of the copy-url issue:

Browse 0.84-platform:

- you can copy the url in the address bar using the 'Ctrl+C' shortcut

- you can NOT copy the url in the address bar using the copy button in the Edit menu

Browse 0.94-platform:

- you can NOT copy the url in the address bar using the 'Ctrl+C' shortcut

- you can NOT copy the url in the address bar using the copy button in the Edit menu

  Changed 21 months ago by erikos

Explanations for the state of art from above:

- the above has nothing to do with the usage of the new entry widget (based on a gtk.Entry) instead of the AddressEntry from the toolkit in Browse's url entry (verified by using a gtk.Entry in Browse platform 0.84, the result was the same as above)

- when we hit the copy button in the Edit menu (in 0.94 and 0.84) we do end up in the following  call which uses the  browser widget as input source, there was a slight change in _get_command_manager due to the introduction of tabbed browsing but this has no effect on the fact that we apply the copy on the browser widget only

- in 0.94 we end up in the  method as well when we use the shortcut 'Ctrl+c' this is because we did add the  accelerator in the sugar-toolkit, and since we only handle the browser widget in this call, the copying of the url in the address bar using the shortcut does NOT work in 0.94 neither (this can be easily verified when removing the accelerator in the toolkit)

  Changed 21 months ago by erikos

I have been thinking a bit about it: once we connect to the 'clicked' signal of the button it is up to the activity to handle the copying/pasting to the clipboard.

In Write for example you can only use the copy and paste functionality when you select text on the widget. If you select something in the title entry for example you can not copy it. In Chat you can copy/paste from/into the title entry but here the edit toolbar and therefore the copy button with the accelerator is not present.

One fix would be to check in the callback for the 'clicked' signal from the copy-button if the entry has been focused and copy the selected text then to the clipboard. A rough patch would be:

diff --git a/edittoolbar.py b/edittoolbar.py
index f0cdb1a..a93d9b3 100644
--- a/edittoolbar.py
+++ b/edittoolbar.py
@@ -110,8 +110,38 @@ class EditToolbar(activity.EditToolbar):
         command_manager.doCommand('cmd_redo', None, None)
 
     def __copy_cb(self, button):
-        command_manager = self._get_command_manager()
-        command_manager.doCommand('cmd_copy', None, None)
+        if self._activity._primary_toolbar.entry.has_focus():
+            bounds = self._activity._primary_toolbar.entry.get_selection_bounds()
+            self._copy_to_clipboard(self._activity._primary_toolbar.entry.get_chars(bounds[0], bounds[1]))
+        else:
+            command_manager = self._get_command_manager()
+            command_manager.doCommand('cmd_copy', None, None)
+
+    def _copy_to_clipboard(self, text):
+        clipboard = gtk.Clipboard()
+        targets = gtk.target_list_add_uri_targets()
+        targets = gtk.target_list_add_text_targets(targets)
+        targets.append(('text/x-moz-url', 0, 0))
+
+        clipboard.set_with_data(targets,
+                                self.__clipboard_get_func_cb,
+                                self.__clipboard_clear_func_cb, text)
+
+    def __clipboard_get_func_cb(self, clipboard, selection_data, info, data):
+        uri_targets = \
+            [target[0] for target in gtk.target_list_add_uri_targets()]
+        text_targets = \
+            [target[0] for target in gtk.target_list_add_text_targets()]
+
+        if selection_data.target in uri_targets:
+            selection_data.set_uris([data])
+        elif selection_data.target in text_targets:
+            selection_data.set_text(data)
+        elif selection_data.target == 'text/x-moz-url':
+            selection_data.set('text/x-moz-url', 8, data)
+
+    def __clipboard_clear_func_cb(self, clipboard, data):
+        pass
 
     def __paste_cb(self, button):
         command_manager = self._get_command_manager()

We would need to as well do something similar for the paste button.

in reply to: ↑ 7 ; follow-up: ↓ 18   Changed 21 months ago by godiard


The above would solve the issues above.

Hmm, no. There are 100 ways to create links with other objects inside, and the web developers will use all (I know, I was one). In DOM land, the only way to know if you are in a link is walk until go to the document object.

  Changed 20 months ago by manuq

  • cc manuq added

  Changed 9 months ago by humitos

  • priority changed from Unspecified by Maintainer to Normal

Copy URL entry: #3596

in reply to: ↑ 15   Changed 6 weeks ago by manuq

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

Replying to godiard:



The above would solve the issues above.


Hmm, no. There are 100 ways to create links with other objects inside, and the web developers will use all (I know, I was one). In DOM land, the only way to know if you are in a link is walk until go to the document object.

This was solved by humitos in commit c3b6dd91 for bug #3455 . Browse now checks for links using WebKit.HitTestResultContext.LINK, not looking at nodes.

Note: See TracTickets for help on using tickets.