Need specific event to notify fullZoom changes

NEW
Unassigned

Status

()

Core
DOM: Events
10 years ago
11 months ago

People

(Reporter: glazou, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

10 years ago
If an extension to FF3 changes the fullZoom factor directly through the markupDocumentViewer, the zoom factor in the UI (and in other codes/extensions dealing with it) is not updated because we have no way to notify these codes the fullZoom factor changed. I suggest a specific event to do that, dispatched when |nsIMarkupDocumentViewer.SetFullZoom()| succeeds.
(Reporter)

Comment 1

10 years ago
taking bug
Assignee: nobody → daniel

Comment 2

10 years ago
This is related to bug 303405.
(Reporter)

Comment 3

10 years ago
Created attachment 302576 [details] [diff] [review]
fix #1

Comment 4

10 years ago
Comment on attachment 302576 [details] [diff] [review]
fix #1

>+  nsEvent evt(PR_TRUE, NS_FULLZOOMCHANGED);
>+  // XXX Dispatching to |window|, but using |document| as the target.
>+  evt.target = mDocument;
We should limit this hack, hopefully to load/unload events which can't be changed without
breaking lots of sites.

>+    { &nsGkAtoms::onfullzoomchanged,             { NS_FULLZOOMCHANGED, EventNameType_All }},
Do we need to add support for the new attribute? Would it be enough to be able to use
window.addEventListener("FullZoomChanged", ...);

If yes, then it is enough to just call
nsContentUtils::DispatchTrustedEvent(mDocument, mDocument->GetWindow(), "FullZoomChanged", PR_TRUE, PR_TRUE, nsnull);
in nsDocumentViewer.cpp. No need to create new nsEvent type or anything.

Comment 5

10 years ago
And perhaps the event shouldn't bubble. That way event listeners (in chrome)
for bubble phase wouldn't get called when subwindows get zoomed.
But if someone really wants to catch also those events, using capturing
listener would allow that.

Comment 6

10 years ago
Still one comment. I'd say it is better to dispatch the event after viewmanager update batch. Usually better to avoid running scripts while in a vm batch (though that is safe).
(Reporter)

Comment 7

10 years ago
Created attachment 302590 [details] [diff] [review]
fix #2 in answer to smaug
Attachment #302576 - Attachment is obsolete: true
So my general comments:

1)  I don't like adding yet another completely undocumented event.  We need to have some documentation to go with this (devmo?).
2)  I don't like adding yet another non-gecko-namespaced event, though perhaps it's OK.
3)  Executing script in the middle of CallChildren might or might not be a good idea.  I guess the call on the prescontext might do that anyway, so this stuff might already be prone to crashes and exploits (since we hold no strong ref to the prescontext or presshell here).... But making it easier to hook in evil code seems like a dubious thing security-wise.
(Reporter)

Comment 9

10 years ago
(In reply to comment #8)
> So my general comments:
> 
> 1)  I don't like adding yet another completely undocumented event.  We need to
> have some documentation to go with this (devmo?).

Sure. No checkin about this w/o doc.

> 3)  Executing script in the middle of CallChildren might or might not be a good
> idea.  I guess the call on the prescontext might do that anyway, so this stuff
> might already be prone to crashes and exploits (since we hold no strong ref to
> the prescontext or presshell here).... But making it easier to hook in evil
> code seems like a dubious thing security-wise.

I chose this way of doing because it was really unintrusive in terms of existing code. We can of course dispatch the event after the completion of CallChildren, and that will solve the multiple-event-per-window problem.

Updated

9 years ago
Blocks: 468835
(Assignee)

Updated

9 years ago
Product: Core → Core Graveyard
The event could be called MozFullZoomChanged and it should be sent asynchronously
or using a script runner.
Assignee: daniel → nobody
Component: GFX → DOM: Events
Product: Core Graveyard → Core
QA Contact: general → events
You need to log in before you can comment on or make changes to this bug.