1434210 - White page is displayed when using a long search query

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P2)

Description

[Sorina Florean [:sflorean]]

Environment: 
Device: Google Pixel(Android 8.0), HTC 10 (Android 7.0), Nexus 5 (Android 6.0.1);
Build: Nightly 60.0a1 (2018-01-29);
Prerequisites: Google is default search engine.

Steps to reproduce:
1. Launch Fennec and go to planet.mozilla.org;
2. Long tap on a word to bring text selection menu;
3. Select "Select all";
4. Select "Google Search".

Expected result:
A page is displayed with the results.

Actual result:
A white page is displayed.

Notes: 
- Reproducible also with Bing, Yahoo, Wikipedia and other search engines;
- On Chrome is not reproducible.

Comment 1

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1434210-1.0-Expose-text-selection-trimming-as-a-.patch

By exposing the selection trimming as a utility function, we can reuse it in Fennec to avoid code duplication.

Comment 2

[Eugen Sawin [:esawin]]

Attachment: 0002-Bug-1434210-2.0-Trim-selection-text-for-search.-r-sn.patch

Currently, we don't sanitize or trim the selection string for search input, we should probably reuse desktop's trimming for it.

Comment 3

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1434210-1.0-Expose-text-selection-trimming-as-a-.patch

Fixed style.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8948694 [details] [diff] [review]
0001-Bug-1434210-1.0-Expose-text-selection-trimming-as-a-.patch

Review of attachment 8948694 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, Eugen!

::: toolkit/modules/BrowserUtils.jsm
@@ +476,5 @@
>        .sameTypeRootTreeItem.QueryInterface(Ci.nsIInterfaceRequestor)
>        .getInterface(Ci.nsIDOMWindow);
>    },
>  
> +  trimSelection(aSelection, aMaxLen) {

Can you please add a docstring for this new function? If you're feeling extra generous, can you also add one for getSelectionDetails?

Comment 5

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1434210-1.1-Expose-text-selection-trimming-as-a-.patch

Added docstrings.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd331b961374
[1.1] Expose text selection trimming as a utility function. r=mconley
https://hg.mozilla.org/integration/mozilla-inbound/rev/f6bb29f3883b
[2.0] Trim selection text for search. r=snorp

Comment 7

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/fd331b961374
https://hg.mozilla.org/mozilla-central/rev/f6bb29f3883b

Comment 8

[Sorina Florean [:sflorean]]

Verified as fixed on latest Nightly build.
Device: HTC Desire 860(Android 6.0.1) and Nexus 6P(Android 8.1.0). Tested with different search engines and after performing the steps from description the search results was displayed.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Seems like a pretty low-risk patch. Should we consider backporting it to Beta for 59 as well?

Comment 10

[Eugen Sawin [:esawin]]

Comment on attachment 8949824 [details] [diff] [review]
0001-Bug-1434210-1.1-Expose-text-selection-trimming-as-a-.patch

Approval Request Comment
[Feature/Bug causing the regression]: Search queries from text selection
[User impact if declined]: Long search queries from text selection may leave the tab in an unresponsive state showing a blank page
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Comment 0
[List of other uplifts needed for the feature/fix]: Patch 2 from this bug
[Is the change risky?]: No
[Why is the change risky/not risky?]: We only apply trimming of the search query based on desktop Firefox rules
[String changes made/needed]: None

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8949824 [details] [diff] [review]
0001-Bug-1434210-1.1-Expose-text-selection-trimming-as-a-.patch

Low risk, verified in nightly, so sure, let's take this for beta 12.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/4442114ced57
https://hg.mozilla.org/releases/mozilla-beta/rev/7a29ddefeab9

Comment 13

[Oana Horvath [:ohorvath]]

Verified on Beta 59.0b13.
Device: 
HTC 10 (Android 8.0)
LG G4 (Android 5.1)