1489307 - Port Bug 1255270 "Favicon request doesn't timeout, or close when related window is closed" to SeaMonkey

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Frank-Rainer Grahl (:frg)]

+++ This bug was initially created as a clone of Bug #1255270 +++

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160303134406

Steps to reproduce:

The Firefox does background request to load favicon.ico from the current web page. Unexpectedly, the connection is not terminated when the website window is closed which gives an server owner an ability to send data regularly to the browser and figure out when the transmission fails indicating the browser close. 

It is also possible to measure pings using ACKs. Theoretically, could it be possible to recognize mobile device geological position from a small list of alternative positions based on ping flow? Remember that the pinging is possible as long as browser is on.


Actual results:

No timeout on favicon connections.


Expected results:

Timeout on favicon connections.

Comment 1

[Frank-Rainer Grahl (:frg)]

Attachment: 1489307-faviconservice-V2.patch

This is more or less a 1:1 patch. The places favicon parts did already go in with Bug 1378089 so all this is needed is just to call them. 

I was unable to test the code in loadFavIcon. I do not use a proxy. If someone does please check. 

I believe the previous code in loadFavIcon was not entirely correct. The favicon loading is/was asynchronous for some time. So bailing out if the icon was in the failed favicon cache probably never happened. I moved this into a callback which should only do the setAttribute when successful. This is just a good guess but it did this in the feedwriter code so should be correct.

I removed setting the icon in feedwriter.js. This does usually not work. The icon is only set if it was previously fetched with exactly the url in the region.properties which never happend. I can re-add this but it is imho a waste. See part 2 why.

2.49 would need some extensive rebasing. I am quite sure if we have the same sec problem as Fx with no mobile client so I don't think it would be worth it. If you do not agree please let me know and i will do a 2.49 fix.

Comment 2

[Frank-Rainer Grahl (:frg)]

Attachment: 1489307-rss-cleanup.patch

aol and digg are now dead. Feedly uses a new api and an account. This would need new code. This leaves yahoo. I don't have an account there but it should still work.

I updated the http links to https too. We can not rename all of them because some names are used in toolkit. Should we add a message to localizers or rename internal ones and/or take this part to a new bug?

Comment 3

[Ian Neal]

Comment on attachment 9007066 [details] [diff] [review]
1489307-faviconservice-V2.patch

Thanks r/a=me

Comment 4

[Ian Neal]

Comment on attachment 9007068 [details] [diff] [review]
1489307-rss-cleanup.patch

I would have this as a separate bug.
As far as I am aware you still have:
browser.contentHandlers.types.1.title=feedly
browser.contentHandlers.types.1.uri=https://feedly.com/i/subscription/feed/%s
browser.contentHandlers.types.2.title=Inoreader
browser.contentHandlers.types.2.uri=https://www.inoreader.com/?add_feed=%s
browser.contentHandlers.types.3.title=Netvibes
browser.contentHandlers.types.3.uri=https://www.netvibes.com/subscribe.php?url=%s

as well as Yahoo. There is also Newsblur, The Old Reader, G2 Reader and Feeder but not sure if they work the same way...

Comment 5

[Frank-Rainer Grahl (:frg)]

Attachment: 1489307-faviconservice-V2a.patch

The patch unfortunately caused a regression. The favicon was only loaded on the first url of a page. Navigating the page cleared it. The callback was never called and loadFavIcon was actually not proxy only as I first thought. I still think this might need to be changed but setting the attribute this way worked before so save refactoring this for a later time.

Only one change which removes the added callback in loadFavIcon compared to the first patch.

Comment 6

[Frank-Rainer Grahl (:frg)]

Comment on attachment 9007068 [details] [diff] [review]
1489307-rss-cleanup.patch

Will be taken to a new Bug

Comment 7

[Ian Neal]

Comment on attachment 9007454 [details] [diff] [review]
1489307-faviconservice-V2a.patch

r/a=me

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/313ef1e61609
Port Bug 1255270 [Favicon request doesn't timeout, or close when related window is closed] to SeaMonkey. r=IanN

Comment 9

[Frank-Rainer Grahl (:frg)]

https://hg.mozilla.org/releases/comm-esr60/rev/ac6fd31c4b23a638c3d221c448f15df3191b61b4