My favorites | Sign in
Project Home Wiki Issues
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 2448: Firebug HTML panel encoding display and editing
  Back to list
Status:  Fixed
Owner:  sroussey
Closed:  Nov 2009
Cc:  johnjbar...@johnjbarton.com, odva...@gmail.com


Sign in to add a comment
 
Project Member Reported by sroussey, Nov 2, 2009
Purpose of code changes on this branch:

To fix issues #2439, #2438, # 2435, #2359, #2075, #1588, #1138, #838 and
supersedes #2299.

Notes:
- I generate entity lists for substitution and regexps for splits. I did a
little testing early on and found the regexp splits to work the fastest.
- When possible, I wrote in shortcuts to skip unneeded processing if some
options are not on.

When reviewing my code changes, please focus on:
- UI changes. I have a few extra options in the HTML tab menu which could
be done differently.
- Whitespace gets a grey border, and entities get a dark red border.
- domplate reuse. See:
http://groups.google.com/group/firebug-working-group/browse_thread/thread/4d465e00562d14b8
- Thoughts on putting all the encoding and stringify type stuff into a
separate file. All the entity declarations clutter up the lib.js file. In
version 2 of this patch I plan to move it.

Known bugs:
- When showing whitespace and editing a text node that is not a
TextElement's text node, after editing, the rendering in Firebug does not
get updated correctly. Typing \t for example, in a text node will make it
look like whitespace if it is a leaf element aka TextElement. But if you
try it elsewhere, it gets rendered as a tab. This is what I was talking
about here:
http://groups.google.com/group/firebug-working-group/browse_thread/thread/2dcb89c4fe0ac81b

Completeness:
- I'm guessing I am about 70% of the way there. 
- I haven't checked much for side-effects (things like the Copy HTML menu
item, etc).
- Need to verify that it actually fixes each of the issues listed above.
- Code cleanup and commenting...


Nov 8, 2009
Project Member #20 sroussey
Hopefully a version of Firebug that people can install...
firebug-1.5b3-sr-1.xpi
835 KB   Download
Nov 9, 2009
Project Member #24 sroussey
Cleaned up and consolidated old comments:

Issues addressed:

 Issue #2470 
 Issue #2453 
 Issue #2439 
 Issue #2438 
 Issue #2435 
 Issue #2359 
 Issue #2299 
 Issue #2250 
 Issue #1980 
 Issue #1588 
 Issue #1138 
 Issue #838 

Notes:

Entity handling is done by creating regexps to do splits (fastest), they get created
on demand when needed and cached. While most conversions are straight forward, the
conversions for text nodes are done in such a way that we can see which entity list
was responsible for the purpose of marking up the entities with different styles. The
Show White Space option and the Text As Source option style their entities/whitespace
somewhat differently.

XHTML/MathML/SVG: 

I personally don't see much use of these since they have to be served from the
webserver in such a way that IE chokes on such documents. Even so, some people do use
them, and thus they don't use IE. Likely they use Firefox, and thus Firebug if they
are going to debug these documents. Firebug should at least not fail as much. I've
added in some support, a little better handling of errors, and alter the UI to
address them directly.

When HTML5 comes around and it becomes a lot easier to use svg/mathml, additional
direct support will be warranted. But for now, the goal was to work better and fail
less. 

Firefox puts CSS styling stuff in attributes internally for MathML. This causes a
number of problems. The quick fix was to remove them sight, when serializing to a
string (copy and editing), and when showing in the html panel. Editing nodes with
such styles could lead to their loosing of their styles. There should be more
research on this, and likely an issue for fbug and one for Mozilla.


In errors.js: 

I found that if I had chromebug set to break on errors, I would stop on a line that
was checking for the existance of FBTrace, but it was undefined. I suppose this is
why it is bracketed by try/catch but I dislike breaking on it, so I use typeof to see
if it is undefined first (which the only way to see if a variable is defined or not
without throwing an exception).

browserOverlay.xul, lib/entities.js:

I moved entity definitions to their own file.


Multiple files, renaming and adding of options:

escapeHTML - changed to escapeForCss/escapeForTextNode/escapeForAttribute since the
rules are slightly different depending on why it is being called. Attributes, since
they are always displayed with double quotes would show a double quote in entity
form, for example. Completely removing escapeHTML forced finding all the cases.

showWhitespaceNodes - the way the code was, it was simply dealing with text nodes,
not text nodes that contained only whitespace (which Fx has an attribute for, if I
remember correctly). As such, it was showing whitespace inside text nodes, so I
renamed it showTextNodesWithWhitespace. I needed this to keep things clear as to what
things meant, but I no longer need the mental crutch, though it might be helpful to
someone in the future.

showTextNodesAsSource - open to a better name. This references the feature, both in
display and in editing to treat the text as if it were source code. Thus show
entities, and if a user types <b> they mean bold. With this option off, if a user
types <b> it means as the browser would see it and converts it to &lt;b&gt;.


lib.js: 

versionChecker/appInfo -- I didn't add these - and don't know where they come from.
Any thoughts?
Some functions were added to check if a node is xhtml/svg/mathml.

Being xml derived, xhtml, mathml, svg all have issues with bad markup. So some error
handling was added, since bad markup is the default while someone is editing a node
and getting updates live. Some error handing was added that essentially cancels
updates until it is conformant.

SVG/MathML tags self close if empty. Also in html panel

I found a list of void tags and updated selfClosingTags/invisibleTags

html.js

domplate stuff for textnode/textelement changed to use getNodeTextGroups which
returns an array of strings and style information. Removal of WhitespaceNode.

Split out TextNodeEditor from AttributeEditor (there was a todo comment from Joe to
do so). TextNodeEditor deals with text nodes that have sibling elements by keeping
track of their position via a range (this is in part because as people type html, the
range will keep track of the text nodes and elements that the user is creating as
they type, and this range could be in the middle of the parent elements children).



editNode made to recognize the type of node and use a specialized editor. Just a hook
for now. All editors are just aliases of HTMLEditor. Possible extension point. A
MathML extension could use a ASCIIMathML based editor, for example, by replacing
Firebug.HTMLPanel.Editors.MathML. 


htmlLib.js

added hasCommentChildren


reps.js

remove MathMl -moz-XXX attributes that are really css

getNodeText => getNodeTextGroups - return an array of string/class info for styling
in domplate

getContextMenuItems takes care of noticing svg/mathml nodes for the context menus

firebug-1.5b3-sr-1.xpi
835 KB   Download
sroussey-firebug-issue-2448-v15.patch
84.7 KB   View   Download
Nov 9, 2009
Project Member #25 johnjbar...@johnjbarton.com
>lib.js: 
>
>versionChecker/appInfo -- I didn't add these - and don't know where they come from.
>Any thoughts?

I don't understand why you are asking about this.

We'll discuss this patch in our call tomorrow. I'm inclined to put it in 1.5b4.
Nov 9, 2009
Project Member #26 sroussey
I had a couple lines added in the patch that I didn't write and I guess were a result of updating my svn 
repo. They don't exist in the current patch. 
Nov 10, 2009
Project Member #27 sroussey
Slight change to the console injector to vary insertion type based on type of
document (HTML vs XHTML/SVG). This way, normal HTML users don't see the console as
<html:div>...
sroussey-firebug-issue-2448-v16.patch
85.5 KB   View   Download
firebug-1.5b3-sr2.xpi
835 KB   Download
Nov 11, 2009
Project Member #28 sroussey
Slight changes:

- added TextAsSource to all language property files (in English) as placeholder.
- added the Edit SVG / Edit MathML parts to the HR locale.
- add back a getNodeText method to try and get plain text of converted text node for
backward compat for other extensions that might be looking for it. I couldn't find
any in the fbug SVN, but they could be out there... otherwise it is not used.

Status?
sroussey-firebug-issue-2448-v17.patch
94.9 KB   View   Download
Cc: johnjbar...@johnjbarton.com
Nov 11, 2009
Project Member #29 johnjbar...@johnjbarton.com
I have the v16 patch on my other machine, but the test suite was broken for another
reason. I will try again tomorrow. 
Nov 11, 2009
Project Member #30 johnjbar...@johnjbarton.com
Honza, I guess the locale changes should *not* be committed or ? Note that there are
two kinds of locale changes. One is a "template" where a line with "HTML" is
duplicated twice and the HTML substituted with SVG and MATHML.  I suppose these are
ok. The other is the english  place holder for one message, which I guess not
needed/desired?
Cc: odvarko
Nov 12, 2009
Project Member #31 sroussey
Let me know, it's pretty easy to fix/change.
Nov 12, 2009
Project Member #32 johnjbar...@johnjbarton.com
R4821 on branches/firebug1.5 has the patch from comment 27. It will be in 1.5b4

Nov 12, 2009
Project Member #33 johnjbar...@johnjbarton.com
R4821 on branches/firebug1.5 has the patch from comment 27. It will be in 1.5b4

Status: Commit
Nov 13, 2009
Project Member #34 odva...@gmail.com
> Honza, I guess the locale changes should *not* be committed ?
Yes, Only the en-US file should be changed.

I am following these rules when getting new strings translated:
1) FB developers should change only the en-US locale files, which stand like a
template for all translated locales.
2) If some string (its value) is changed, change also its key so, translators can see
there is something new to translate.
3) Update from BZ: always get all current translations (even for not released
locales) from BZ before uploading new version. So, no translation made since the last
upload is lost. The only dir, which is *not* synced with BZ is locale/en-US.

Note that I am always using only files downloaded from BZ (except en-US), since they
always have correct encoding (it's easy to break the encoding with editors).

It's ok to keep the locale changes for now (if all the encodings is correct). They
will be overwritten by BZ versions anyway - as soon as I am syncing.

> Note that there are two kinds of locale changes. One is a "template"
> where a line with "HTML" is duplicated twice and the HTML substituted
> with SVG and MATHML. I suppose these are ok.
So, I guess the following:

EditHTMLElement=Edit HTML...
EditSVGElement=Edit SVG...
EditMathMLElement=Edit MathML...

CopySVG=Copy SVG
CopyMathML=Copy MathML

> The other is the english  place holder for one message, which I guess not
> needed/desired?
This one?
ShowTextNodesAsSource=Text As Source

This one is used for a new option in the HTML panel. Not sure why this shouldn't be
needed. We need that option, right?

Honza


Nov 13, 2009
Project Member #35 johnjbar...@johnjbarton.com
Honza, maybe you can put those instructions on the wiki?

Regarding ShowTextNodesAsSource I only meant we did not want the placeholder in the
.properties files of every locale.
Nov 13, 2009
Project Member #37 sroussey
OK, one note then: I changed the name of EditElement to EditHTMLElement. I wouldn't 
have done that had I known the process...

Was:

EditElement=Edit HTML...

now:

EditHTMLElement=Edit HTML...

And these were added:
ShowTextNodesAsSource=Text As Source
EditSVGElement=Edit SVG...
EditMathMLElement=Edit MathML...
CopySVG=Copy SVG
CopyMathML=Copy MathML

ShowTextNodesAsSource needs translation. The others I had guessed (though I think 
they are valid).

Changing the UI to show the type of markup for the context menu isn't strictly 
needed, but I made the calls to edit HTML/SVG/MathML separate such that an extension 
could provide its own implementation (I was thinking ASCIIMathML as an example, if 
it were reversible). Therefore, I figured if the editor could be different, the UI 
should reflect that. And it might put the idea in some developers head upon seeing 
that…

Nov 18, 2009
Project Member #38 johnjbar...@johnjbarton.com
This is one of 26 fixes in Firebug 1.5X.0b4. Please try the new release and
let us know if this problem is solved.
http://getfirebug.com/releases/firebug/1.5X
Status: Fixed
Jun 5, 2012
Project Member #39 sebastia...@gmail.com
(No comment was entered for this change.)
Labels: fixed-1.5-b4
Sign in to add a comment

Powered by Google Project Hosting