Search for link text context menu item should pay attention to selection

VERIFIED FIXED in Firefox 31

Status

()

VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: philor, Assigned: vtsatskin)

Tracking

Trunk
Firefox 31
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox29 affected, firefox30- affected, firefox31 verified, relnote-firefox 31+)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
If you select just part of the text of a link, instead of the bug 970746 search for the link's entire text, we should search for the selection.

That's the easy part. Harder: part of the link text, and some non-link text selected; that's probably still search-the-selection. Non-link text selected, none of the link, target is the link; that should be the link text, not the selection.
(Assignee)

Comment 1

5 years ago
Posted patch 985824.patchSplinter Review
Attachment #8397460 - Flags: review?(philringnalda)
Attachment #8397460 - Flags: review?(bmcbride)
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
Attachment #8397460 - Attachment is patch: true
Attachment #8397460 - Flags: review?(philringnalda)
Attachment #8397460 - Flags: review?(bmcbride) → review+
(Reporter)

Comment 2

5 years ago
Comment on attachment 8397460 [details] [diff] [review]
985824.patch

On second thought, you're probably right to have not believed me about "text selected outside the link, but click target is the link, search the link text" since after playing with it for a while, that feels like it's more likely to happen to someone by accident who would be surprised by what they searched for than to be something someone would intentionally do, change their mind about what to search for after selecting some text.
Attachment #8397460 - Flags: feedback+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b816c8efe28b
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 31
The following changeset is now in Firefox Nightly:

> b816c8efe28b Bug 985824 - Search for link text context menu item should pay attention to selection. r=Unfocused

Nightly Build Information:

        ID: 20140402030201
 Changeset: 4941a2ac0786109b08856738019b016a6c5a66a6
   Version: 31.0a1
      TBPL: https://tbpl.mozilla.org/?rev=4941a2ac0786
       URL: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central

Download Links:

>         Linux x86: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.linux-i686.tar.bz2
>      Linux x86_64: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.linux-x86_64.tar.bz2
> Linux x86_64 ASAN: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.linux-x86_64-asan.tar.bz2
>               Mac: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.mac.dmg
>             Win32: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.win32.installer.exe
>             Win64: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.win64-x86_64.installer.exe

Previous Nightly Build Information:

        ID: 20140401030203
 Changeset: 1417d180a1d8665b1a91b897d1cc4cc31e7980d4
   Version: 31.0a1
      TBPL: https://tbpl.mozilla.org/?rev=1417d180a1d8
       URL: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-01-03-02-03-mozilla-central

Comment 6

5 years ago
Is this applicable to Firefoxbeta30? 
Or, too late?
status-firefox29: --- → affected
status-firefox30: --- → affected
status-firefox31: --- → fixed
tracking-firefox30: --- → ?
No reason to track this, but if it's a low risk of regression we could take an uplift to this week's Beta (this is the last week we'll take on new, non-tracked, speculative work on Beta).
tracking-firefox30: ? → -
Flags: needinfo?(vtsatskin)
(Assignee)

Comment 8

5 years ago
(In reply to Lukas Blakk [:lsblakk] from comment #7)
> No reason to track this, but if it's a low risk of regression we could take
> an uplift to this week's Beta (this is the last week we'll take on new,
> non-tracked, speculative work on Beta).

Differing to Unfocused as I don't think I have the knowledge to make that call. 

@Unfocused: when you do make this decision, I'd be interested in learning how you go about making it.
Flags: needinfo?(vtsatskin) → needinfo?(bmcbride)
(In reply to Lukas Blakk [:lsblakk] from comment #7)
> No reason to track this, but if it's a low risk of regression we could take
> an uplift to this week's Beta (this is the last week we'll take on new,
> non-tracked, speculative work on Beta).

No, I wouldn't uplift this to Beta.
Flags: needinfo?(bmcbride)
(In reply to Valentin Tsatskin [:vt] from comment #8)
> @Unfocused: when you do make this decision, I'd be interested in learning
> how you go about making it.

Think of this as cost/benefit, and a risk analysis. Aurora has higher risk and higher bar than Nightly, Beta has an even higher risk and therefore higher bar to reach. So, if something lands in Beta, what is the impact if something goes wrong? Beta has a far larger audience, and a far less technical audience. There's also the issue of how hard it will be to backout. There are Beta builds every week, as opposed to any day there's a change on Aurora - so a backout can take much longer to get into a Beta build than Aurora.

For this specific bug, we have a couple of things to consider:
* The behaviour is a rare edge case - from what we know, very few people will have issues without this patch, so the benefit will be low
* And because we believe so few people will have been hitting this fixed behaviour, it would have gotten far less real-world testing. So the risk is higher. We believe the tests cover the code portion of the patch well enough, but I'm more concerned about the UX point of view for this.
(In reply to Blair McBride [:Unfocused] from comment #10)
Great, thanks for the detailed response. We shall watch it as it rides the trains, I'm going to flag this for a release note so that we're giving people a heads up to try this and know about it.
relnote-firefox: --- → ?
relnote-firefox: ? → 31+
Tagging verifyme for QA to do some exploratory testing around this.
Keywords: verifyme
This issue is verified fixed on Firefox 31 Beta 1 (Build ID: 20140610163407), using:
 * Windows 7 64-bit [1],
 * Ubuntu 13.10 64-bit [2],
 * Mac OS X 10.9.2 [3].

[1] Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0
[2] Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Firefox/31.0
[3] Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:31.0) Gecko/20100101 Firefox/31.0
Status: RESOLVED → VERIFIED
status-firefox31: fixed → verified
Keywords: verifyme

Updated

5 years ago
Duplicate of this bug: 1025837

Updated

5 years ago
Duplicate of this bug: 1038733
You need to log in before you can comment on or make changes to this bug.