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

Status: VERIFIED FIXED

(Firefox for iOS :: General, defect)

Description

[Nick Nguyen [:osunick]]

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.

Comment 1

[Aaron Train [:aaronmt]]

Nathanael, is this the same bug as bug 1282790?

Comment 2

[Nathanael Alcock]

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.

Comment 3

[Nathanael Alcock]

Attachment: Pull request

Comment 4

[Farhan Patel [:farhan]]

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?

Comment 5

[Nathanael Alcock]

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.

Comment 6

[Farhan Patel [:farhan]]

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.

Comment 7

[Nathanael Alcock]

https://github.com/mozilla/firefox-ios/commit/bb811fd3318d9d5aa44aa30e8278461478330441

Comment 8

[Stephan Leroux [:sleroux]]

v5.x b136a03

Comment 9

[Simon Basca [:SimonB]]

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.