#3066 closed defect (fixed)
BrowseV124 regressions
Reported by: | tonyforster | Owned by: | erikos |
---|---|---|---|
Priority: | Normal | Milestone: | |
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 (19)
comment:1 Changed 12 years ago by erikos
comment:2 Changed 12 years 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
comment:3 Changed 12 years 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
comment:4 Changed 12 years ago by tonyforster
btw probably not relevant to ctrl-C but the 1.75 is a mechanical keyboard
comment:5 follow-up: ↓ 6 Changed 12 years 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?
comment:6 in reply to: ↑ 5 Changed 12 years 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.
comment:7 follow-up: ↓ 15 Changed 12 years 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.
comment:8 Changed 12 years 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.
comment:9 Changed 12 years 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>)
comment:10 follow-up: ↓ 11 Changed 12 years 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.
comment:11 in reply to: ↑ 10 Changed 12 years 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
comment:12 Changed 12 years 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
comment:13 Changed 12 years 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)
comment:14 Changed 12 years 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.
comment:15 in reply to: ↑ 7 ; follow-up: ↓ 18 Changed 12 years 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.
comment:16 Changed 12 years ago by manuq
- Cc manuq added
comment:17 Changed 11 years ago by humitos
- Priority changed from Unspecified by Maintainer to Normal
Copy URL entry: #3596
comment:18 in reply to: ↑ 15 Changed 10 years ago by manuq
- Resolution set to fixed
- Status changed from assigned to closed
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.
This is known, the activities page should note that 124 is only compatible with 0.94.
Can you elaborate here?
Do you have steps to reproduce? It seems to work here.