608266 - [Regression] Can't open a link in new tab from the awesome panels

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

Regression of the functionnality added in bug 590517

Steps to reproduce:
 * click on the urlbar
 * long tap on one of the awesome result

Actual result:
 * a context menu with "Share Link"

Expected result:
 * a context menu with "Open Link In New Tab", "Share Link"


The patch also add a small changes for not highlighting the categories in the history view.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 486911 [details] [diff] [review]
Patch


>diff --git a/chrome/content/content.js b/chrome/content/content.js

> ContextHandler.registerType("link-saveable", function(aState, aElement) {
>   let protocol = aState.linkProtocol;
>   return (protocol && protocol != "mailto" && protocol != "javascript" && protocol != "news" && protocol != "snews");
> });
> 
>+ContextHandler.registerType("link-openable", function(aState, aElement) {
>+  let protocol = aState.linkProtocol;
>+  return (protocol && protocol != "mailto" && protocol != "javascript" && protocol != "news" && protocol != "snews");
>+});
>+
> ContextHandler.registerType("link-shareable", function(aState, aElement) {
>   return Util.isShareableScheme(aState.linkProtocol);
> });
> 
> ["image", "video"].forEach(function(aType) {
>   ContextHandler.registerType(aType+"-shareable", function(aState, aElement) {
>     if (aState.types.indexOf(aType) == -1)
>       return false;

Use the Array style to register "link-saveable" and "link-openable", like "image" and "video", since the impl is the same.

>diff --git a/themes/core/browser.css b/themes/core/browser.css

>-autocompleteresult:hover:active,
>+#popup_autocomplete autocompleteresult:hover:active,
> placelist placeitem:hover:active:not([selected="true"]),
>-historylist placeitem:hover:active:not([selected="true"]):not([class="history-item-title"]),
>-remotetabslist placeitem:hover:active:not([selected="true"]):not([class="remotetabs-item-title"]),
>+historylist autocompleteresult:hover:active:not([selected="true"]):not([class="history-item-title"]),
>+remotetabslist autocompleteresult:hover:active:not([selected="true"]):not([class="remotetabs-item-title"]),
> .autocompleteresult-selected {
>   background-color: #8db8d8;
> }

What's this stuff?

r+ with the Array style change and removing the CSS if it's not needed.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Any ideas for testing this?

Comment 3

[Wesley Johnston (:wesj)]

Attachment: Tests

Sorry to step on your toes a bit viv. finkle's comment just got me curious if this was hard for some reason. I'm still not sure what it means even. This tests for context clicks on all three pages, and updates tests for the "link-openable" stuff.

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #3)
> Created attachment 487449 [details] [diff] [review]
> Tests
> 
> Sorry to step on your toes a bit viv. finkle's comment just got me curious if
> this was hard for some reason. I'm still not sure what it means even. This
> tests for context clicks on all three pages, and updates tests for the
> "link-openable" stuff.

No worries, I appreciate tests, especially when someone else wrote them :)

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

http://hg.mozilla.org/mobile-browser/rev/00af481bc83f
http://hg.mozilla.org/mobile-browser/rev/2b2493b0a968

Comment 6

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Verified:

Mozilla/5.0 (Maemo;Linux armv71; rv:2.0b8pre) Gecko/20101104 Firefox/4.0b8pre Fennec/4.0b3pre
Mozilla/5.0 (Android; Linux armv71; rv2.0b8pre) Gecko/20101104 Firefox/4.0b8pre Fennec/4.0b3pre