Closed Bug 538813 Opened 10 years ago Closed 10 years ago

de-uglify about:cache and about:cache-entry

Categories

(Core :: Networking: Cache, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b2

People

(Reporter: steffen.wilberg, Assigned: steffen.wilberg)

References

()

Details

Attachments

(9 files, 7 obsolete files)

73.77 KB, image/png
Details
35.72 KB, image/png
Details
65.48 KB, image/png
Details
90.70 KB, image/png
Details
11.76 KB, application/xhtml+xml
Details
3.82 KB, application/xhtml+xml
Details
37.60 KB, image/png
Details
65.19 KB, image/png
Details
32.93 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
Attached patch v1 (obsolete) — Splinter Review
I hammered about:cache and about:cache-entry into the new about: layout.
I sliced overly long URLs by using "word-wrap: break-word".
about:cache now uses tables instead of spaces for layout.
Both now produce properly indented code.
about:cache uses the XHTML mime type, thus fixing bug 403677.

The attached patch is 99% done, and I want to save my work.
Component: Networking → Networking: Cache
QA Contact: networking → networking.cache
some test urls:

http://www.mozilla.org/about/owners.html
(displays long "request-Accept-Encoding:" heading)
about:cache-entry?client=HTTP&sb=1&key=http://www.mozilla.org/about/owners.html

http://www.mediaevent.de/wp-content/themes/mediaevent/images/thislu.gif
(very long Server header)
about:cache-entry?client=HTTP&sb=1&key=http://www.mediaevent.de/wp-content/themes/mediaevent/images/thislu.gif

http://www.feedblitz.com/i/32/16320.bmp
(Data size: 0 bytes, so no hex dump)
about:cache-entry?client=HTTP&sb=1&key=http://www.feedblitz.com/i/32/16320.bmp

This is the most stubborn url, because it doesn't wrap unless properly beaten with "word-wrap: break-word;" and restricting the width of the containing element:
http://www.google-analytics.com/__utm.gif?utmwv=4.6.5&utmn=1764755025&utmhn=meiert.com&utmcs=UTF-8&utmsr=1280x1024&utmsc=24-bit&utmul=en-us&utmje=1&utmfl=10.0%20r42&utmcn=1&utmdt=XHTML%20und%20der%20richtige%20MIME-Typ%20%E2%80%93%20Artikel%20von%20Jens%20Meiert&utmhid=70851642&utmr=http%3A%2F%2Fwww.google.de%2Furl%3Fsa%3Dt%26source%3Dweb%26ct%3Dres%26cd%3D1%26ved%3D0CAgQFjAA%26url%3Dhttp%253A%252F%252Fmeiert.com%252Fde%252Fpublications%252Farticles%252F20041004%252F%26rct%3Dj%26q%3Dxhtml%2Bmime%2Btype%26ei%3DwTpJS8LODIj__AbUo9WPAg%26usg%3DAFQjCNFKtY5uh29az2pywsNha450uwi6gA&utmp=%2Fde%2Fpublications%2Farticles%2F20041004%2F&utmac=UA-209576-1&utmcc=__utma%3D207987086.1090625301.1263090366.1263090366.1263090366.1%3B%2B__utmz%3D207987086.1263090366.1.1.utmcsr%3Dgoogle%7Cutmccn%3D(organic)%7Cutmcmd%3Dorganic%7Cutmctr%3Dxhtml%2520mime%2520type%3B

about:cache-entry?client=HTTP&sb=1&key=http://www.google-analytics.com/__utm.gif?utmwv=4.6.5&utmn=1764755025&utmhn=meiert.com&utmcs=UTF-8&utmsr=1280x1024&utmsc=24-bit&utmul=en-us&utmje=1&utmfl=10.0%20r42&utmcn=1&utmdt=XHTML%20und%20der%20richtige%20MIME-Typ%20%E2%80%93%20Artikel%20von%20Jens%20Meiert&utmhid=70851642&utmr=http%3A%2F%2Fwww.google.de%2Furl%3Fsa%3Dt%26source%3Dweb%26ct%3Dres%26cd%3D1%26ved%3D0CAgQFjAA%26url%3Dhttp%253A%252F%252Fmeiert.com%252Fde%252Fpublications%252Farticles%252F20041004%252F%26rct%3Dj%26q%3Dxhtml%2Bmime%2Btype%26ei%3DwTpJS8LODIj__AbUo9WPAg%26usg%3DAFQjCNFKtY5uh29az2pywsNha450uwi6gA&utmp=%2Fde%2Fpublications%2Farticles%2F20041004%2F&utmac=UA-209576-1&utmcc=__utma%3D207987086.1090625301.1263090366.1263090366.1263090366.1%3B%2B__utmz%3D207987086.1263090366.1.1.utmcsr%3Dgoogle%7Cutmccn%3D%28organic%29%7Cutmcmd%3Dorganic%7Cutmctr%3Dxhtml%2520mime%2520type%3B
Attached patch v1.1 (obsolete) — Splinter Review
Attachment #420925 - Attachment is obsolete: true
Attached patch patch v1.2 (obsolete) — Splinter Review
This should be it. Boris, would you review this, or should I ask biesi?
Or does Darin still reviews?

Should I move the style tags into separate stylesheets? The other "about" pages are not consistent here, some use tags, some stylesheets.
Attachment #420947 - Attachment is obsolete: true
Attachment #420972 - Flags: review?(bzbarsky)
Why are we switching from HTML to XHTML here?

Can I see screenshots of the affected pages with this patch applied?

In general, I can review this, I think.
> Why are we switching from HTML to XHTML here?
about:cache already has a XHTML1.1 doctype. Using "text/html" sends it to the tag-soup parser instead of the XML parser.
If you're worried about xml parsing errors, I can change the doctype to HTML instead. I haven't seen any errors though.
about:cache-entry already uses the application/xhtml+xml mime type.
Attached image about:cache screenshot (obsolete) —
about:cache is wider than it needs to be, but it fits with the following pages.
Attached image about:cache?device=memory screenshot (obsolete) —
This is the screenshot of about:cache entry of the overlong url from comment 1.

I'd like to shrink-wrap the left <th> column, but haven't found a good way yet. If I don't limit the width of the table the way I do, it gets very wide and the url overflows the <body> to the right.
> Using "text/html" sends it to the tag-soup parser instead of the XML parser.

Yes, I'm well aware.

> If you're worried about xml parsing errors

That, and also the lack of user control over character encodings in XML.  It's been a major problem for us recently in some of our internally-generated questions.

Why is it a problem to send data with the XHTML1.1 doctype to a tag-soup parser?  The spec explicitly allows for it...  In any case, the right change there is probably to use the HTML5 doctype.
> I'd like to shrink-wrap the left <th> column, but haven't found a good way yet.

"width: -moz-fit-content" doesn't do what you want?  Note that you'd have to not use "table-layout: fixed" if you want to do that, obviously.

> If I don't limit the width of the table the way I do, it gets very wide and
> the url overflows the <body> to the right.

Sure.  Why is limiting its width a problem?  You're setting it to 100%, right?  Why is it narrower than the viewport in the screenshots?

Another question.  Why are you putting |display: block| on your <td>s?
Using the XHTML doctype is kind of useless, if it is ignored by the parser because of the text/html mime type. I'm happy to use the HTML5 doctype instead for both documents.

(In reply to comment #12)
> Sure.  Why is limiting its width a problem?  You're setting it to 100%, right? 
I'm ok with using "width: 12em;", but am not sure how long those headers can get. The longest I came across is "request-Accept-Encoding:".

> Why is it narrower than the viewport in the screenshots?
It's narrower because I use body { display: table; }.
That means it shrink-wraps horizontally to the hexdump at the bottom, which is the desired visual effect. I want to avoid horizontal scrollbars at all costs.
Screenshot upcoming.

> Another question.  Why are you putting |display: block| on your <td>s?
Unneeded copy-and-paste. Will remove.
The doctype is ALWAYS ignored by our parser, for both HTML and XHTML.  We never do anything with doctypes, basically, other than loading internal subsets and maybe importing entities for a few well-known doctypes.

> I'm ok with using "width: 12em;"

I thought you were talking about the table's width being limited...

> not sure how long those headers can get

Arbitrarily long.  Sites can just make them up.

> It's narrower because I use body { display: table; }.

Ah, I see.

> which is the desired visual effect.

Why is it desired to have a narrow body even if there's space for a wider one, exactly?
> "width: -moz-fit-content" doesn't do what you want?  Note that you'd have to
> not use "table-layout: fixed" if you want to do that, obviously.
I didn't know about -moz-fit-content. This is how it looks like with 
th { width: -moz-fit-content; } and without table-layout: fixed.

Without table { width: 100%; }, the second table with the http headers moves to the left, but that doesn't help much.

With table-layout: fixed, the columns are 50% wide each.
I just noticed that I don't need
    th { white-space: nowrap;}
anymore and can replace it by
    th { word-wrap: break-word; }
which breaks very long headers instead of spilling over the right column.

There's space for a wider body, but I don't want to make it arbitrarily wide just because of a long url, and having the rest of the content shifted to the left, as seen in my last screenshot.

Most of the about: pages, like about: or about:about have body { max-width: 50em; }. Those needing wider bodies, like about:license, use max-width: 80%.

All in all, I'm not unhappy with the way about:cache-entry looks like.
> This is how it looks like with th { width: -moz-fit-content; } and without
> table-layout: fixed.

Odd.  Did you also drop the "tbody { display: table; }"?  If not, why did it stop shrink-wrapping?

Can you perhaps attach the generated HTML+CSS here to this bug (just copy-paste out of view-source)?

> Most of the about: pages, like about: or about:about have body { max-width:
> 50em; }. Those needing wider bodies, like about:license, use max-width: 80%.

Right, and those scale with the window width, whereas the setup in the attached patches forces this page to be a particular number of characters in the users's 
default monospace size.  In particular, not scaling with the window width, right?

In any case, a more serious problem is that if someone sends a longer-than-12em header, what happens?
(In reply to comment #17)
> > This is how it looks like with th { width: -moz-fit-content; } and without
> > table-layout: fixed.
> Odd.  Did you also drop the "tbody { display: table; }"? 
No.

> If not, why did it stop shrink-wrapping?
Because of that very url, and similar ones I encountered. They have very little ampersands, hyphens and dashes and wrap very poorly therefore. It works much better with other urls like this:
http://www.google.de/url?sa=t&source=web&ct=res&cd=1&ved=0CAoQFjAA&url=http%3A%2F%2Fwiki.whatwg.org%2Fwiki%2FFAQ&rct=j&q=html5+mime+type&ei=05BLS-HUA9D7_AbGzYWZAg&usg=AFQjCNHqin9N0kS_lcxTvM3z4FmWKxX9lQ

By the way, width:-moz-fit-content had the same effect as not specifying width at all in my experiments.

> Can you perhaps attach the generated HTML+CSS here to this bug (just 
> copy-paste out of view-source)?
Will do. Ctrl+S works just fine, is there a difference to View Source?

> > Most of the about: pages, like about: or about:about have body { max-width:
> > 50em; }. Those needing wider bodies, like about:license, use max-width: 80%.
> 
> Right, and those scale with the window width, whereas the setup in the
> attached patches forces this page to be a particular number of characters in
> the users's default monospace size.  In particular, not scaling with the 
> window width, right?
about:cache has <body class="aboutPageWideContainer>, which means max-width: 80%.
about:cache-entry does not scale. But contrary to pages like about:licence, about:credits with lots of free flowing text, it doesn't benefit much from the increased width. The page consists mostly of short lines, except the url, the "file on disk" value, and a few overly long http header values.

> In any case, a more serious problem is that if someone sends a longer-
> than-12em-header, what happens?
With the attached patch, it spills over to the right.
With th { word-wrap: break-word; } from comment 16, the header wraps.
In this case the url wraps badly as well. Remove table-layout:fixed, and *poof*, the table explodes horizontally.
> By the way, width:-moz-fit-content had the same effect as not specifying width
> at all in my experiments.

Well, table cells _do_ shrink-wrap their contents by default, more or less.  That is what you asked for, right?

> Will do. Ctrl+S works just fine, is there a difference to View Source?

Nope.

Will take a look at that generated html/css tonight.  Thanks for attaching it!
In this case the url wraps good. Removing "table-layout: fixed;" has no effect on this page while "th { width: 12em; }" is still specified.
Attached patch patch v2.0 (obsolete) — Splinter Review
- switched to text/html and html5 doctype per comment 11
- used th { word-wrap: break-word; } per comment 16
- on about:cache, the "List Cache Entries" isn't displayed anymore if the number of entries is zero
- moved the style definitions to two new style sheets
- added some color :-)
- redesigned about:cache?device=disk etc.:
Key/Data size/Fetch count/Last modified/Expires is now displayed in a row instead of on top of each other. I'd like to make the columns sortable in a follow-up bug.
Attachment #420972 - Attachment is obsolete: true
Attachment #421117 - Attachment is obsolete: true
Attachment #421118 - Attachment is obsolete: true
Attachment #426904 - Flags: review?(bzbarsky)
Attachment #420972 - Flags: review?(bzbarsky)
Attached image about:cache screenshot
Attached patch patch v2.1 (obsolete) — Splinter Review
Added packaging the winstripe css files to pinstripe, like about.css.
Removed some <tt> leftovers.
Fixed minor bitrot.
Attachment #426904 - Attachment is obsolete: true
Attachment #427976 - Flags: review?(bzbarsky)
Attachment #426904 - Flags: review?(bzbarsky)
Attached patch patch v2.2Splinter Review
* Switched to <th> tags and use thead in preparation for sortable columns in  about:cache?device=disk etc. (I have a patch ready for that...)
* Fixed a silly XXX comment
* Fixed minor jar.mn bitrot
Attachment #427976 - Attachment is obsolete: true
Attachment #436578 - Flags: review?(bzbarsky)
Attachment #427976 - Flags: review?(bzbarsky)
Comment on attachment 436578 [details] [diff] [review]
patch v2.2

r=bzbarsky.  Sorry for the lag; reading through all that HTML is a huge pain.  :(
Attachment #436578 - Flags: review?(bzbarsky) → review+
Pushed http://hg.mozilla.org/mozilla-central/rev/c626a2ff4d1b
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3b2
Yow. The colors!

Please note, for the future, that it would be best to involve UX in the design of in-product pages. Either beforehand, or by requesting ui-review.
Yeah, but now UX can play with it and file bugs...
These pages used to be so hideous that nobody could possibly look at them for more than 5 seconds, let alone notice bugs like e.g. "Expires" showing "1970-01-01 01:00:00".
(In reply to comment #29)
> Yow. The colors!

Filed bug 576814

PS - thanks Steffen, nice job.
Apparently the theme stuff hasn't been reviewed at all. Specifying background colors without fitting text colors is bogus.
Adding the push link, for tracking purposes (such as theme authors):
http://hg.mozilla.org/mozilla-central/rev/a283e2c69cf9
P.s., the aboutCacheEntry.css also has a 'text-align: right;' that wants to be fixed.
You need to log in before you can comment on or make changes to this bug.