Closed Bug 608266 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: vingtetun, Assigned: vingtetun)

Details

(Whiteboard: [fennec-checkin-postb2])

Attachments

(2 files)

Attached patch PatchSplinter Review
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.
Attachment #486911 - Flags: review?(mark.finkle)
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.
Attachment #486911 - Flags: review?(mark.finkle) → review+
Whiteboard: [fennec-checkin-postb2]
Any ideas for testing this?
Attached patch TestsSplinter Review
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.
(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 :)
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.