Reporter should record the character encoding used to display the page

RESOLVED FIXED in mozilla1.9beta1

Status

enhancement
RESOLVED FIXED
14 years ago
7 months ago

People

(Reporter: jruderman, Assigned: raccettura)

Tracking

unspecified
mozilla1.9beta1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

()

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

14 years ago
Some odd behavior on web sites depends on what character encoding Firefox decided to use to display the page.  For example, a problem might appear only when the page is interpreted as being UTF-8.  It would be nice if reporter.mozilla.org told me the encoding in addition to telling me the URL.
Posted patch client side patch (obsolete) — Splinter Review
Needs appropriate server side changes.
Attachment #209260 - Flags: review?
Assignee

Comment 2

14 years ago
Asa, I'd appreciate your opinion on this one.  As well as anyone else you think would be good to chime.

It's a reasonable idea, but I'm not sure we need this.  I'd like to see some evidence that this is widespread enough for it to help.

I'm against a situation where we start capture an insane amount of data, for very seldom use.  Especially data we can get ourselves (information about the page itself).

If this is something that would help identify a large number of all rendering issues, then yes, this may be worth while.

Otherwise, you should note there is a problem via reports in reporter, take a look at the reported pages, and file a bug in bugzilla with an appropriate test case and note the encoding there.  Soon reporter will be able to point to bugzilla bugs related to that hostname, which will further facilitate this.

reporter isn't an alternative to bugzilla.  It's simply a conduit for users to communicate with developers without the complexities of a system like bugzilla and needing to gather information.  The information we gather is about the client at the moment in time (when visiting the page) so we can reproduce the problem.

What pushes me towards saying it's unneeded is that it's server side metadata. i.e. anyone checking the page to confirm the issue can find it out very easily.  If it were a client side option/configuration, then I'd be more inclined to take the data, as it wouldn't be something we could gather otherwise.  For example, if knowing intl.charset.default would help in many cases, then it would be something to include.

I am actively coding screenshot support, which will allow users to show us the problem.  This would help in identification of actual issues as well.

It's not clear to me that there is a real burden by not capturing the page encoding.  Perhaps there is.
It isn't server side data in some cases - the user may have specified a charset manually, leading to the page being displayed incorrectly. The value captured is the charset being used to display the page, not the charset specified by the page.
Reporter

Comment 4

14 years ago
When a page doesn't specify a charset, Firefox picks one based on all kinds of wacky heuristics, including the frequency distribution of bytes in the page (which might change slightly) and the charset of the linking page.
Assignee

Comment 5

14 years ago
Ok, I'm about 80% agreeing on this addition.

Gavin, the patch looks fine (I need to do the server side obviously).  

I'm working on screenshot support as well.  I'd like to time this bug to land together with that.  

A few things to consider here:
- Are we capturing the interpreted encoding?  Or the page encoding?  I presume interpreted.
- If the encoding is coming from external sources (i.e. from the page), I'd prefer that the case be the same (always upper, or always lower).  It's not critical, but would be nice.  If it's set internally, whatever the browser does is fine.

Reassign->gavin as he's got a patch
Assignee: robert → gavin.sharp
(In reply to comment #5)
> - Are we capturing the interpreted encoding?  Or the page encoding?  I presume
> interpreted.

Correct. It's what the browser is using to display the page.

> - If the encoding is coming from external sources (i.e. from the page), I'd
> prefer that the case be the same (always upper, or always lower).  It's not
> critical, but would be nice.  If it's set internally, whatever the browser does
> is fine.

The case should be consistent, since this is the browser's internal representation of the charset.
Assignee

Comment 7

14 years ago
That's exactly what I wanted. :-D
This mostly needs server-side work, so it shouldn't really be assigned to me.
-> Default owner
Assignee: gavin.sharp → robert
Assignee

Comment 9

13 years ago
Gavin: if you don't mind, file a webtools:reporter bug for the server side.  I've got it in the back of my mind anyway.

I'd rather keep them separate so I can see what's done, and what needs to be done.
Assignee

Comment 10

13 years ago
This is a little late, and we don't support this on the server, but we could land it since the server will ignore what it doesn't understand... then when we upgrade the server, it will start recording charset of reports.

Gavin, care to chime in on this plan?
Flags: blocking1.8.1?
Assignee

Comment 11

13 years ago
Comment on attachment 209260 [details] [diff] [review]
client side patch

I've reviewed this
Attachment #209260 - Flags: review?
Sounds like a fine plan to me.
Assignee

Comment 13

13 years ago
Comment on attachment 209260 [details] [diff] [review]
client side patch

r=me

We likely want to add the results to the report detail for viewing on completion.
Attachment #209260 - Flags: review? → review+
Assignee

Updated

13 years ago
Flags: blocking1.8.1?
Assignee

Comment 14

13 years ago
Comment on attachment 209260 [details] [diff] [review]
client side patch

We should also take this for 1.8.1 if we can.  No string change, we can leave this out of the details view.  It's not really important.
Attachment #209260 - Flags: approval1.8.1?

Comment 15

13 years ago
Comment on attachment 209260 [details] [diff] [review]
client side patch

We are closing in on the RC1 code freeze and are only taking bugs that match the criteria for 181 approvals:

http://developer.mozilla.org/devnews/index.php/2006/08/28/firefox-2gecko-181-approvals-2/

Thanks for putting this together and since it is on the trunk we'll pickup in a future release.
Attachment #209260 - Flags: approval1.8.1? → approval1.8.1-
Assignee

Updated

13 years ago
Blocks: 352905
Assignee

Comment 16

12 years ago
Updated to fix bitrot.  I could have sworn this was checked in on the trunk a while ago.  My mistake.
Attachment #209260 - Attachment is obsolete: true
Attachment #287272 - Flags: review?
Assignee

Updated

12 years ago
Attachment #287272 - Flags: review? → review?(gavin.sharp)
Comment on attachment 287272 [details] [diff] [review]
Update Gavin's Patch

>Index: resources/content/reporter/reportWizard.js

>     'buildconfig':      gParamBuildConfig,
>     'language':         gParamLanguage,
>     'email':            gParamEmail,
>+    'charset':          gCharset,
>     'sysid':            sysId
>   };

If this patch had more context (-u8 next time? :) you'd see a "SOAP Params" comment above this block that should probably be changed.

r=me, though I'm assuming you've tested it, because I haven't.
Attachment #287272 - Flags: review?(gavin.sharp) → review+
Assignee

Updated

12 years ago
Attachment #287272 - Flags: approvalM9?
Attachment #287272 - Flags: approval1.9?
Comment on attachment 287272 [details] [diff] [review]
Update Gavin's Patch

a=beltzner for M9, but we won't hold release for this
Attachment #287272 - Flags: approvalM9?
Attachment #287272 - Flags: approvalM9+
Attachment #287272 - Flags: approval1.9?
Attachment #287272 - Flags: approval1.9+
Assignee

Comment 19

12 years ago
(In reply to comment #17)
> r=me, though I'm assuming you've tested it, because I haven't.

Did, looks fine just like the other patch.

(In reply to comment #18)
> (From update of attachment 287272 [details] [diff] [review])
> a=beltzner for M9, but we won't hold release for this
> 

Ditto.  I can checkin later this week when I have time to be on the hook for a few hours.
I changed "// SOAP params" to just "// params".

Checking in extensions/reporter/locales/en-US/chrome/reportResults.dtd;
/cvsroot/mozilla/extensions/reporter/locales/en-US/chrome/reportResults.dtd,v  <--  reportResults.dtd
new revision: 1.3; previous revision: 1.2
done
Checking in extensions/reporter/resources/content/reporter/report.xhtml;
/cvsroot/mozilla/extensions/reporter/resources/content/reporter/report.xhtml,v  <--  report.xhtml
new revision: 1.3; previous revision: 1.2
done
Checking in extensions/reporter/resources/content/reporter/reportWizard.js;
/cvsroot/mozilla/extensions/reporter/resources/content/reporter/reportWizard.js,v  <--  reportWizard.js
new revision: 1.31; previous revision: 1.30
done
Checking in extensions/reporter/resources/content/reporter/reporterOverlay.js;
/cvsroot/mozilla/extensions/reporter/resources/content/reporter/reporterOverlay.js,v  <--  reporterOverlay.js
new revision: 1.10; previous revision: 1.9
done
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M9

Updated

7 months ago
Product: Other Applications → Other Applications Graveyard
You need to log in before you can comment on or make changes to this bug.