655747 - Add BrowserUtils.jsm for commonly used browser-related code

Status: RESOLVED WONTFIX

(Toolkit :: General, defect)

Description

[:Gavin Sharp [email: [e-mail]]]

To be shared between SeaMonkey/Fennec/Firefox and other browser-like apps. The immediate use case is bug 482921 (see bug 482921 comment 24).

One alternative to this would be to add this code to the "browser" binding (as suggested in the comment above Firefox's current FillInHTMLTooltip), but I'm not sure it fits in there very well. I've also not fully investigated which other code could live in this module, so that should probably be investigated further before going down this path.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

Neil: do you think SeaMonkey would want to make use of what is essentially the Firefox copy of FillInHTMLTooltip? I noticed it's a lot more complicated than the one you seem to be using currently.

Neil/mfinkle: can you think of other things that would be useful to put here?

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

Oops, forgot to qrefresh.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

(sigh)

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 531097 [details] [diff] [review]
patch

The hard part is going to be keeping BrowserUtils from becoming a kitchen sink of code. I mean the current code is not useful to Fennec. Maybe BrowserUtils.jsm is too generic?

TooltipUtils.jsm ?

Comment 5

[:Gavin Sharp [email: [e-mail]]]

The idea was to fill it with code that Fennec currently has forked/copied - hoping you had some ideas as to whether such code existed.

Comment 6

[[e-mail]]

(In reply to comment #1)
> Neil: do you think SeaMonkey would want to make use of what is essentially
> the Firefox copy of FillInHTMLTooltip? I noticed it's a lot more complicated
> than the one you seem to be using currently.
Well, we honour newlines in tooltips, and we also have the fix for bug 264001, so we couldn't take it as it stands. (Other differences are that we also vastly simplified the loop, and we also didn't add tooltips for form validation, and we don't have the XLink or SVG stuff for some reason.)

> Neil/mfinkle: can you think of other things that would be useful to put here?
I had a skim through but I couldn't find anything, sorry.

Comment 7

[[e-mail]]

Comment on attachment 531097 [details] [diff] [review]
patch

Trying someone else who might have some ideas...

Comment 8

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #6)
> Well, we honour newlines in tooltips

(we have bug 358452 for that, fwiw)

Comment 9

[Philip Chee]

Comment on attachment 531097 [details] [diff] [review]
patch

>>> Neil: do you think SeaMonkey would want to make use of what is essentially
>>> the Firefox copy of FillInHTMLTooltip? I noticed it's a lot more complicated
>>> than the one you seem to be using currently.
>> Well, we honour newlines in tooltips, and we also have the fix for bug 264001, so we couldn't take it as it stands. (Other differences are that we also vastly simplified the loop,

> and we also didn't add tooltips for form validation, and we don't have the XLink or SVG stuff for some reason.)

We have Bug 601091 (Port FillInHTMLTooltip changes from firefox) that would have given us those, but it seems stuck in someone's review queue.

Ah I notice in that bug that I pointed out:

> <http://dev.w3.org/html5/spec/rendering.html#the-title-attribute-0>
> <http://www.w3.org/TR/html5/elements.html#the-title-attribute>

> (we have bug 358452 for that, fwiw)
I knew there had to be a bug for this.

At the moment there is only one item in BrowserUtils.jsm and I don't think the Mobile stuff that you plan to include is relevant to SeaMonkey. I don't think it's worth taking at the moment (even if we can get our simplified loop into the toolkit version), but it's always a good idea to share code in the long term.

Can't comment on the actual patch as I don't have much of a clue in this area. Sorry.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Sounds like perhaps there isn't enough justification for this at this point. I guess I should focus on adding this code to browser.xml instead.

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #10)
> Sounds like perhaps there isn't enough justification for this at this point.
> I guess I should focus on adding this code to browser.xml instead.

How about tabbrowser.xml? Why add it to browser.xml?

I think I am saying "we don't want that code in mobile" :)

Comment 12

[:Gavin Sharp [email: [e-mail]]]

tabbrowser isn't in toolkit. The code is needed in toolkit/components/viewsource and browser/. It applies generally to all browsers - mobile is just special because it doesn't show tooltips (because it doesn't have hovering).

Comment 13

[:Gavin Sharp [email: [e-mail]]]

We'll be exploring things like this in the future I'm sure, but this ended up not being the right solution!