Bug 31611 – Web Inspector: turn off line wrapping in resource views for Resources and Scripts panels

• Component: Web Inspector (Deprecated)
• Status: RESOLVED INVALID

[Patrick Mueller]

Currently the resource views in the Resources and Scripts panels, for instance, viewing a JavaScript file, display the contents with the lines wrapped.  There is no option to not wrap.  There *should* be an option to non-wrap, and perhaps, not wrapping should be the default.

This is at least my personal preference.  I never wrap lines while editing source code, so the wrapped lines for JavaScript are very unnatural to me.

[Patrick Mueller]

Setting white-space:nowrap as a style for the <td>'s that contain the source seems to be enough to turn the line wrapping off.  So, it seems like a CSS rule based on a class on the table itself is one way to make this happen.  We can then toggle the style in the table to wrap/not-wrap the <td>'s via the rule.

Not sure WHERE to put the wrap/not-wrap option.  One complication is that it's needed for both the Resources and Scripts panel; I don't think we want two different ways to turn this on or off, or different locations of the trigger, depending on what panel you're on. 

An option 'button' next to the console button would probably work for now.  We could also use the context-menu support that was added a while back - though there is a discovery problem with that, I think.

Perhaps it's time we started thinking about a preferences panel, or popup.  Perhaps something like the console, in the same area as the console.  Only one of the console or preferences would ever be displayed at a time, or neither.

[Pavel Feldman]

> Perhaps it's time we started thinking about a preferences panel, or popup. 
> Perhaps something like the console, in the same area as the console.  Only one
> of the console or preferences would ever be displayed at a time, or neither.

What if we do it as a status bar toggle button in the meanwhile?

[Patrick Mueller]

(In reply to comment #2)
> What if we do it as a status bar toggle button in the meanwhile?

Yes, I'm motivated to do *something* before waiting for a preferences panel to be built!  Either a status bar toggle button or a context menu.  Context menu has the advantage of not requiring an image.

I opened Bug 31622 for discussion on the preference panel idea.

[Pavel Feldman]

(In reply to comment #3)
> (In reply to comment #2)
> > What if we do it as a status bar toggle button in the meanwhile?
> 
> Yes, I'm motivated to do *something* before waiting for a preferences panel to
> be built!  Either a status bar toggle button or a context menu.  Context menu
> has the advantage of not requiring an image.
> 

We do not control popup in web inspector yet. There was a plan on introducing one. There even is a Popup class. It'll take much more time to implement it than to make Tim do some imagery though :)

> I opened Bug 31622 for discussion on the preference panel idea.

[Patrick Mueller]

(In reply to comment #4)
> We do not control popup in web inspector yet. There was a plan on introducing
> one. There even is a Popup class.

Right-mouse-button (or ctrl-click) over the line numbers in a resource view does display a fakey popup though.  That's what I was actually thinking of reusing.  Perhaps we should just wait till we have proper popups.

Will use a status bar toggle button for now.

What do people think about changing the default to not wrap?

[Pavel Feldman]

(In reply to comment #5)
> (In reply to comment #4)
> > We do not control popup in web inspector yet. There was a plan on introducing
> > one. There even is a Popup class.
> 
> Right-mouse-button (or ctrl-click) over the line numbers in a resource view
> does display a fakey popup though.  That's what I was actually thinking of
> reusing.  Perhaps we should just wait till we have proper popups.
> 

Yep, it is Popup.js we recently introduced. It was supposed to be used for popup menu as well.

> Will use a status bar toggle button for now.
> 
> What do people think about changing the default to not wrap?

It would improve debugging experience I guess, but would add some confusion when looking at minified js. I'd say I don't care.

[Patrick Mueller]

(In reply to comment #6)

> > What do people think about changing the default to not wrap?
> 
> It would improve debugging experience I guess, but would add some confusion
> when looking at minified js. I'd say I don't care.

You make it sound like looking at minified js isn't already confusing!

[Timothy Hatcher]

We have many more pressing issues with minified js already. Making it show on one line might be less confusing, since it shoes the true nature of the file.

I think no-wrap by default. Maybe even no toggle for now and see what people say.

[Patrick Mueller]

(In reply to comment #8)
> I think no-wrap by default. Maybe even no toggle for now and see what people
> say.

Sounds good to me.  I'll do this for the moment.  I think I've seen people have code committed without closing the bug - shall we commit this change (simple one), but leave the bug open to add the toggle?

For toggling, I noticed that the table for the source is actually created in WebCore/html/HTMLViewSourceDocument.cpp, as near as I can tell.  The way I was thinking of doing this was adding a class to the table itself, indicating it was the source container - class name "webkit-source-container".  Then code in SourceFrame.js will toggle an additional class in that element to toggle wrap, and we'd add the CSS rule to the literal CSS toggle it, like

table.webkit-source-container.wrapped > td.webkit-line-content {
   white-space: pre-wrap; // or whatever it is we need here
}

Kind of a pain to have to edit the .cpp file to do this, but seems safer to mark the actual table when it gets created rather than have to guess in SourceFrame.js.  If we want to make the change to the .cpp file, it looks like this is the only "unnamed" element left in the source frame (just checking to see if we wanted to name anything else at the same time for future function).

[Timothy Hatcher]

There is code in SourceFrame.js that injects into the iframe for source code. We do that for many CSS chnages already, and thats where you should inject this change too.

Keepign the bug open seems fine. We just need to clear the review flags on the patch when it lands, to not confuse tools.

[Patrick Mueller]

Created attachment 43772 [details]
some of the required changes - incomplete

Some of the changes required for this fix.

Three changes:

- change the line number cell to use min-width: 31px; without this, the number area narrows for long lines.  I wasn't sure if usage of min-width means we didn't actually need the width style or not.  

- change the line content cell to use white-space: pre; this is the primary fix - lines no longer wrap

- remove an extra line number gutter; turns out there are two added, the one removed here does not scroll when scrolling horizontally; not sure why this one is there

With these changes, the source viewed in the Scripts panel works well (see some caveats below).  However, source viewed from the Resources panel, not so much.  The problem in the Resources panel is that one of the <div>, <table>, <iframe> is set too long, and so you have to scroll to the bottom of the source text to see the horizontal scroll bar.  Actually, this is ok for me - I rarely need to scroll right, and even if you need to, you can do that by selecting and dragging right (and left to scroll back).  But clearly we can't ship it this way.

I spent a bit of time trying to get the Resources panel to work, but was unsuccessful.  Figured I'd try passing the torch to someone more familiar with the guts of [Script,Source,Resource][Panel,Frame,View].js .  I may take another go at it if no one else picks it up.

Caveats:

- the line number area scrolls horizontally with the text.  Just like TextMate.  Unlike BBEdit.  I prefer the BBEdit-style of NOT scrolling the line numbers, but it's not a deal-breaker for me to have this, and I suspect getting the numbers to NOT scroll will be work, if it's even possible.

- in the Resources panel, the HTTP info at the top scrolls vertically with the text.  It already did this, there's no function change here.  But I'm wondering now if it should also be static, and not scroll with the text.  I think i prefer it to scroll, but not a big deal either way.

[Timothy Hatcher]

I thought you might run into issues in Resources panel.

The HTTP info should scroll, otherwise you wont have any room in Docked mode to see the resource. We try to minimize the number of static bars/info for this reason.

Xcode also keeps the line numbers on screen, which I also prefer.

[Timothy Hatcher]

I have been considering removing the iframe for source views. Just use the iframe to syntax highlight HTML and generate the table view (done in native code.) Then clone the iframe's DOM and insert it into an element in the Inspector. Then we have more control over the layout and scrolling behaviour.

[Timothy Hatcher]

This will be obsolete once we switch to SourceFrame2.