IMDB desktop search box can't be added as a search engine

VERIFIED FIXED

Status

()

VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: osunick, Assigned: nalcock)

Tracking

unspecified
Other
iOS

Firefox Tracking Flags

(fxios-v5.0 affected, fxios-v5.1 verified, fxios5.1+)

Details

Attachments

(1 attachment)

48 bytes, text/x-github-pull-request
farhan
: review+
Details | Review | Splinter Review
(Reporter)

Description

2 years ago
On iPad, visit IMDB and try to add the main search box as a search engine in v5.0. An error occurs. If you switch to mobile view, it works properly.
Nathanael, is this the same bug as bug 1282790?
Flags: needinfo?(nalcock)
(Assignee)

Comment 2

2 years ago
This is actually a separate issue with loading the wrong favicon, and then not supporting SVG favicons (which is probably another bug in itself). I'm going to work on a fix.
Flags: needinfo?(nalcock)
(Assignee)

Updated

2 years ago
Assignee: nobody → nalcock
(Assignee)

Comment 3

2 years ago
Created attachment 8770303 [details] [review]
Pull request
Attachment #8770303 - Flags: review?(fpatel)

Updated

2 years ago
status-fxios-v5.0: --- → affected
tracking-fxios: --- → ?
Looks good. Just some nits.

How does this make sure that a SVG favicon isn't requested? By using the existing favicon logic in the browser is it just selecting a different favicon compared to before which this time happens to not be SVG?

Updated

2 years ago
Attachment #8770303 - Flags: review?(fpatel) → review+
(Assignee)

Comment 5

2 years ago
I've just updated it to prioritise picking smaller favicons — do you think this is a better approach, or unnecessary?

Regarding SVG favicons — no, this doesn't address that issue. I've created another bug for that here: https://bugzilla.mozilla.org/show_bug.cgi?id=1286394
I think really that's a separate issue. However, this approach does mean we won't be given an SVG URL to fetch (because SDWebImageManager would have already failed, and thus not set it as the tab's favicon), so it indirectly at least mitigates that case.
Flags: needinfo?(fpatel)
I think its unnecessary. On retina devices the icon is actually 60x60. And on a 3x device it would be 90x90. If we end up choosing a 16x16 icon because it is the smallest it'll be blurry. Best to get whichever one the tab is already using and just use that. It'll get resized and look good on retina. Also with the added benefit that the tab favicon is already cached so reusing it is quicker.
Flags: needinfo?(fpatel)
(Assignee)

Comment 7

2 years ago
https://github.com/mozilla/firefox-ios/commit/bb811fd3318d9d5aa44aa30e8278461478330441
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
tracking-fxios: ? → 5.1+
Whiteboard: [needsuplift]
v5.x b136a03
status-fxios-v5.1: --- → fixed
Whiteboard: [needsuplift]
Tested on 5.1b2 on iPhone 6 Plus (9.3.3) and iPad Air 2 (9.3.3).

IMDB desktop search box can be added / removed as a search engine.
Status: RESOLVED → VERIFIED
status-fxios-v5.1: fixed → verified
You need to log in before you can comment on or make changes to this bug.