Closed Bug 1434210 Opened 2 years ago Closed 2 years ago

White page is displayed when using a long search query

Categories

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

ARM
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 60
Tracking Status
fennec + ---
firefox58 --- wontfix
firefox59 --- verified
firefox60 --- verified

People

(Reporter: sflorean, Assigned: esawin)

Details

Attachments

(2 files, 2 obsolete files)

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.
Assignee: nobody → esawin
tracking-fennec: ? → +
Priority: -- → P2
By exposing the selection trimming as a utility function, we can reuse it in Fennec to avoid code duplication.
Attachment #8948691 - Flags: review?(mconley)
Currently, we don't sanitize or trim the selection string for search input, we should probably reuse desktop's trimming for it.
Attachment #8948692 - Flags: review?(snorp)
Fixed style.
Attachment #8948691 - Attachment is obsolete: true
Attachment #8948691 - Flags: review?(mconley)
Attachment #8948694 - Flags: review?(mconley)
Attachment #8948692 - Flags: review?(snorp) → review+
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?
Attachment #8948694 - Flags: review?(mconley) → review+
Added docstrings.
Attachment #8948694 - Attachment is obsolete: true
Attachment #8949824 - Flags: review+
Pushed by esawin@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/fd331b961374
https://hg.mozilla.org/mozilla-central/rev/f6bb29f3883b
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
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.
Seems like a pretty low-risk patch. Should we consider backporting it to Beta for 59 as well?
Flags: needinfo?(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
Flags: needinfo?(esawin)
Attachment #8949824 - Flags: approval-mozilla-beta?
Attachment #8948692 - Flags: approval-mozilla-beta?
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.
Attachment #8949824 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8948692 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Verified on Beta 59.0b13.
Device: 
HTC 10 (Android 8.0)
LG G4 (Android 5.1)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.