The "Open with" button can be accessed even though is not visible

RESOLVED FIXED

Status

()

Firefox for iOS
General
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: SimonB, Assigned: Maurya Talisetti, Mentored)

Tracking

unspecified
All
iOS

Firefox Tracking Flags

(fxios+, fxios-v4.0 affected, fxios-v5.0 affected, fxios-v6.0 affected, fxios-v7.0 fixed)

Details

(Whiteboard: [good second bug])

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
Build 4.0.0b2

Steps to reproduce:

1. Launch Firefox
2. Open a PDF document in a new tab
3. Scroll up
4. Tap in the top right corner where the "Open with" button was before scrolling
5. Observe that Share menu will open as the buttons hotspot can be taped 

Actual results:
- The "Open with" button can be accessed even though is not visible.

Expected results:
- The "Open with" actions should not be available when the button is hidden.
Mentor: etoop@mozilla.com
Rank: 7
tracking-fxios: ? → +
Whiteboard: [good first bug]

Comment 1

2 years ago
Hi Emily,

I am interested in this bug ans want to resolve this bug. Can you please assign this bug to me? Also could you please give me some guidelines regarding how to solve this bug?

Thankyou
Flags: needinfo?(etoop)
Hi,

Firstly, thanks so much for having a look at this. Please don't hesitate to contact me with any questions.

The Open In view is added to the webViewContainerToolbar, which is inside the webViewContainer, when we detect a PDF has been loaded AND we have an app on our device that can open it. The banner appears at the top of the page, just below the URL bar. It is added to the view on line 2118 of BrowserViewController.swift in a function called addOpenInViewIfNecessary(url:), and removed in removeOpenInView(). The webViewContainerToolbar is hidden and displayed depending on whether or not there is anything inside it.

When we scroll the webview, it scrolls over the top of the webViewContainerToolbar. What is happening here is that, somehow, the Open In button is still pressable despite no longer being visible on the screen. It seems that the webView WKScrollView is propagating the touches down to the toolbar.

There are 2 solutions to this that I can see,

1. Instead of scrolling the webview over the top of the webViewContainerToolbar, we either scroll the webViewContainerToolbar off the screen too, or we hide it when it is no longer visible, so it is no longer available to receive touches.
2. We prevent the touches from being propagated through the WKPDFView & WKScrollView down to the view beneath.

As the hide animation for the webViewContainerToolbar is rather jerky at the moment, I think I would prefer a solution that also fixed that animation jankiness so suggest that you pursue solution 1 initially.
Assignee: nobody → agore1
Status: NEW → ASSIGNED
Flags: needinfo?(etoop)

Comment 3

2 years ago
Its hard to work on....Can you suggest me any easy bug. I am new to MDN and want to contribute. Please help me 

Thanks
Flags: needinfo?(etoop)

Updated

2 years ago
Assignee: agore1 → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(etoop)
Whiteboard: [good first bug] → [good second bug]
Hey,


I've just push a few new bugs into Good First Bugs. Take a peek and see whether any of these take your fancy.

https://bugzilla.mozilla.org/buglist.cgi?cmdtype=runnamed&namedcmd=Good%20First%20Bugs&list_id=13002736
Flags: needinfo?(agore1)

Comment 5

2 years ago
(In reply to Emily Toop (:fluffyemily) from comment #4)
> Hey,
> 
> 
> I've just push a few new bugs into Good First Bugs. Take a peek and see
> whether any of these take your fancy.
> 
> https://bugzilla.mozilla.org/buglist.
> cgi?cmdtype=runnamed&namedcmd=Good%20First%20Bugs&list_id=13002736

Hi Emily,

Link is not showing any results.
Flags: needinfo?(agore1) → needinfo?(etoop)
How about you give this one a go? https://bugzilla.mozilla.org/show_bug.cgi?id=1267241
Flags: needinfo?(etoop)
(Assignee)

Updated

a year ago
Assignee: nobody → maurya1985
Status: NEW → ASSIGNED
(Assignee)

Comment 7

a year ago
Hi Emily,

Not sure if this is a bug, and if worthwhile, but I found that the "Open in..." button doesn't appear on the simulator. It only seems to be appearing on a device.

Should we file a bug for that?
Flags: needinfo?(etoop)
Maurya, the Open In button not appearing on the simulator is not a bug. The Open In button only appears if there is an app on the device that can handle PDF's. By default on the simulator there is not PDF capable app. We do this check by checking for the presence of iBooks, which is installed by default on all devices and so is a good indicator of whether a PDF enabled app is present on the device.

If you need to test this on a simulator, then you can change the check in the OpenInPDFHelper (https://github.com/mozilla/firefox-ios/blob/f2386f93b7e7ed1026c24d935683f6b1470274dc/Client/Frontend/Browser/OpenInHelper.swift#L125) to not perform the canOpenURL check. This will enable the Open In button to appear on the simulator.
Flags: needinfo?(etoop)
(Assignee)

Comment 9

a year ago
Created attachment 8785698 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/2069
Attachment #8785698 - Flags: review?(etoop)
(Assignee)

Comment 10

a year ago
review ping
(Assignee)

Comment 11

a year ago
Hi Emily, would you be able to review this? If you are unavailable, should I ask somebody else to review it?
Flags: needinfo?(etoop)
Flags: needinfo?(etoop)
Attachment #8785698 - Flags: review?(etoop) → review?(sleroux)
Comment on attachment 8785698 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/2069

Looks good! Thanks for picking this up.
Attachment #8785698 - Flags: review?(sleroux) → review+
master https://github.com/mozilla-mobile/firefox-ios/commit/130a1e44bf94c3f42a9a34d3252c3e37c0976182
status-fxios-v6.0: --- → affected
status-fxios-v7.0: --- → fixed
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
(Assignee)

Comment 14

a year ago
Hi Stephan,

Though it's working, I faced some problem which I described in some comments on the PR (in case you missed it). Would you be able to comment on that?

https://github.com/mozilla-mobile/firefox-ios/pull/2069#discussion-diff-76534623
https://github.com/mozilla-mobile/firefox-ios/pull/2069#discussion-diff-76534718
Flags: needinfo?(sleroux)
I've added some responses to the PR!
Flags: needinfo?(sleroux)
You need to log in before you can comment on or make changes to this bug.