757016 - API for changing editor styling after initialization

Status: RESOLVED WONTFIX

(DevTools :: Source Editor, defect, P3)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Sometimes it's useful to have API that allow to access the underlying document used for SourceEditor rendering. The document is hidden within an iframe (there are actually two iframes) 

Having a reference to the document can be useful e.g. for dynamic CSS style modification and/or for dynamic DOM modifications.

Here is a possible workaround:
var doc = this.editor._view._frameDocument;

Both, _view and _frameDocument fields are private so, not safe to use.

Better would be?
var doc = this.editor.getViewDocument();

Honza

Comment 1

[Mihai Sucan [:msucan]]

Why would we need to allow direct access to the DOM of the editor? Can you please list some use-cases?

For styling Orion already allows theme CSS changes on-the-fly - we just need to enable them from within the Source Editor component.

My main concern with giving so much power at once is that it's a footgun. Very easy to break things and it's impossible for us to maintain backwards compatibility - it would be an unsupported API. The editor is entirely responsible for the DOM of its iframe and any unexpected changes we make there can cause breakage.

We need a strong case to allow such API.

Comment 2

[Mihai Sucan [:msucan]]

Since Orion upstream no longer provides at TextView level support for changing the styling of the editor after initialization, I would like to implement this capability in the Source Editor. This would solve the need you have, without allowing direct access to the DOM of the editor and avoiding potential breakage.

(slightly morphing this bug into a valid use case we have discussed. If this not OK for you, please let me know!)

Comment 3

[Rob Campbell [:rc] (:robcee)]

Moving to Source Editor component.

Filter on CHELICERAE.

Comment 4

[Mihai Sucan [:msucan]]

Attachment: proposed patch

This is the proposed patch. It cleans-up how we handle stylesheets in the Source Editor and it brings new APIs to change the theme and for adding custom stylesheets into the editor view, without breaking anything.

Honza: please let me know if this kind of API is sufficient for styling needs, for Firebug's use cases. Also, please note bug 760825 which adds new API specific for easy/quick changing of the editor font size - IIRC you needed something like that.

Looking forward for comments. Thank you!

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 646098 [details] [diff] [review]
proposed patch

(In reply to Mihai Sucan [:msucan] from comment #4)
> Honza: please let me know if this kind of API is sufficient for styling
> needs, for Firebug's use cases.
If I understand correctly CSS customization (on top of the current theme) would be done through SourceEditor.setStylesheets (list of URIs to stylesheets or direct strings). It sounds good to me (I didn't test it).

One of the first Firebug usage of the API would be customizing the color syntax.

> Also, please note bug 760825 which adds new
> API specific for easy/quick changing of the editor font size - IIRC you
> needed something like that.
Yes, exactly. I'll post related comment to that bug report.

Thanks Mihai!
Honza

Comment 6

[Mihai Sucan [:msucan]]

(In reply to Jan Honza Odvarko from comment #5)
> Comment on attachment 646098 [details] [diff] [review]
> proposed patch
> 
> (In reply to Mihai Sucan [:msucan] from comment #4)
> > Honza: please let me know if this kind of API is sufficient for styling
> > needs, for Firebug's use cases.
> If I understand correctly CSS customization (on top of the current theme)
> would be done through SourceEditor.setStylesheets (list of URIs to
> stylesheets or direct strings). It sounds good to me (I didn't test it).

That is correct.

Thanks for the f+!

Comment 7

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 646098 [details] [diff] [review]
proposed patch

+  setStylesheets: function SE_setStylesheets(aStylesheets)

is that really the best way to do this?

Comment 8

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 646098 [details] [diff] [review]
proposed patch

in +  setStylesheets: function SE_setStylesheets(aStylesheets)

+      if (/^[a-z]+:\/\/[^\n\r]+$/.test(aStyle) || /^data:.+/.test(aStyle)) {

document your regexes. These could also be put in a constants so that's a bit more readable.

Comment 9

[Anton Kovalyov (:anton)]

Is this bug still a thing we'd like to do?

Comment 10

[Mihai Sucan [:msucan]]

This was requested for firebug. It seemed like a useful API to allow the editor component users to change editor styling. Does Honza still want to use our editor component? Is this an API he still needs?

Feel free to WONTFIX this bug if you believe we don't need this kind of APIs.

Comment 11

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Mihai Sucan [:msucan] from comment #10)
> Does Honza still want to use our editor component? Is this an API he still needs?
Firebug doesn't need this anymore. The next major version of Firebug (in alpha at this moment)
is based on Code Mirror (CM embedded in Firebug package).

Thanks
Honza

Comment 12

[Anton Kovalyov (:anton)]

Thanks!