Closed Bug 985824 Opened 8 years ago Closed 7 years ago
Search for link text context menu item should pay attention to selection
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.
Attachment #8397460 - Flags: review?(bmcbride) → review+
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+
https://hg.mozilla.org/integration/fx-team/rev/b816c8efe28b Please make sure that future patches include proper commit information per the guidelines below. Thanks! * https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F * https://developer.mozilla.org/en-US/docs/Developer_Guide/Committing_Rules_and_Responsibilities#Checkin_comment
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
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
Is this applicable to Firefoxbeta30? Or, too late?
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).
(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.
(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.
Tagging verifyme for QA to do some exploratory testing around this.
This issue is verified fixed on Firefox 31 Beta 1 (Build ID: 20140610163407), using: * Windows 7 64-bit , * Ubuntu 13.10 64-bit , * Mac OS X 10.9.2 .  Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0  Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Firefox/31.0  Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:31.0) Gecko/20100101 Firefox/31.0
You need to log in before you can comment on or make changes to this bug.