DOMLinkAdded, DOMLinkChanged and DOMLinkRemoved are not fired for stylesheets

NEW
Unassigned

Status

()

Core
DOM: Events
2 years ago
2 years ago

People

(Reporter: mconley, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

It looks as if we don't fire any of these events for stylesheets, at least in my testing.

These events would be useful for bug 1141041 if we decide to keep a cache of available stylesheets in a page on the <xul:browser> in the parent process.

Testcase incoming.
Created attachment 8668030 [details]
MozReview Request: Bug 1210074 - Testcase.

Bug 1210074 - Testcase.

Comment 5

2 years ago
But maybe we should do something lower level for bug 1141041 and deal with this stuff in C++ without any events? Not sure.
(In reply to Olli Pettay [:smaug] from comment #5)
> But maybe we should do something lower level for bug 1141041 and deal with
> this stuff in C++ without any events? Not sure.

I had two plans:

1) Use events, like DOMLinkAdded (or StyleSheetAdded, if turning those events on didn't regress tp5) to message the stylesheet list up to the parent for the parent to cache, so the page style menupopup can synchronously access that list

2) Have the parent send down a message when the menupopup is opened, and dynamically inject menuitems into the Page Style menu on a response, assuming the menupopup is still open when the response arrives.

I kinda liked the first plan better, since there's less chance of a fast flicker in the menupopup when the user opens it with new nodes getting injected after it opens...

Not sure what other low-level options there are. Was there anything in particular you had in mind?
Flags: needinfo?(bugs)

Comment 7

2 years ago
I was just thinking that perhaps HTMLLinkElement could do something like
ownrDoc()->docshell->getInterface(nsITabChild)->SendDOMLinkAdded(params..).

But perhaps allowing the events to fire always wouldn't be that bad. And removing odd special cases
is always good.
However, mutationEvents do cause significant slow down.

I think I'd be ok to remove the special case to not fire events for stylesheet, but bz might have
different opinion.
Flags: needinfo?(bugs)

Updated

2 years ago
Flags: needinfo?(bzbarsky)
So the idea of the current code is to optimize out the event firing for <link rel="stylesheet"> for two reasons:

1)  The initial consumer of this code (the link toolbar) didn't care about those <link>
    elements.
2)  The original consumer was quite slow at processing the events, so extra events were a
    bad idea.

If we want this specifically for page style, then we need to also respond to @title changes on stylesheets, right?
Flags: needinfo?(bzbarsky)
(In reply to Boris Zbarsky [:bz] from comment #8)
> If we want this specifically for page style, then we need to also respond to
> @title changes on stylesheets, right?

That's true - through the DOMLinkChanged event, I suppose.

If this is too much trouble, I don't think it'd be so bad to just fill the menupopup asynchronously.
You need to log in before you can comment on or make changes to this bug.