Mark HTML as UTF-8....

Mark HTML as UTF-8.

NOT FOR COMMIT; EVAL for upstreaming

BUG=http://crbug.com/11957
TEST=as in bug

[Avi (use Gerrit), 2009-07-09 21:17:47]

What do you think of this approach? We need some way of annotating that this is
UTF-8. On Windows, perhaps that's assumed, but on the Mac unmarked HTML is
assumed to be 8859-1, which is rather bad.

[darin (slow to review), 2009-07-09 22:47:48]

Annotating the charset as UTF-8 seems fine to me.  I'm not sure why there
isn't a similar problem on Windows.  Did you investigate what happens on
Windows?
-Darin


On Thu, Jul 9, 2009 at 2:17 PM, <avi@chromium.org> wrote:

> Reviewers: darin,
>
> Message:
> What do you think of this approach? We need some way of annotating that
> this is UTF-8. On Windows, perhaps that's assumed, but on the Mac
> unmarked HTML is assumed to be 8859-1, which is rather bad.
>
> Description:
> Mark HTML as UTF-8.
>
> NOT FOR COMMIT; EVAL for upstreaming
>
> BUG=http://crbug.com/11957
> TEST=as in bug
>
>
> Please review this at http://codereview.chromium.org/149414
>
> SVN Base: http://svn.webkit.org/repository/webkit/trunk/WebCore/
>
> Affected files:
>  M     platform/chromium/PasteboardChromium.cpp
>
>
> Index: platform/chromium/PasteboardChromium.cpp
> ===================================================================
> --- platform/chromium/PasteboardChromium.cpp    (revision 45659)
> +++ platform/chromium/PasteboardChromium.cpp    (working copy)
> @@ -71,6 +71,7 @@
>  void Pasteboard::writeSelection(Range* selectedRange, bool
> canSmartCopyOrDelete, Frame* frame)
>  {
>     String html = createMarkup(selectedRange, 0, AnnotateForInterchange);
> +    html = String("<meta http-equiv=\"Content-Type\" content=\"text/html;
> charset=UTF-8\">") + html;
>     ExceptionCode ec = 0;
>     KURL url = selectedRange->startContainer(ec)->document()->url();
>     String plainText = frame->selectedText();
>
>
>

[jungshik at Google, 2009-07-09 23:22:57]

On 2009/07/09 22:47:48, darin wrote:
> Annotating the charset as UTF-8 seems fine to me.  I'm not sure why there
> isn't a similar problem on Windows.  Did you investigate what happens on
> Windows?
> -Darin

Because CF_HTML (to which we convert Webkit's HTML) is defined to be in UTF-8
:-)  http://msdn.microsoft.com/en-us/library/ms649015(VS.85).aspx

BTW, I wonder if using a 'UTF-8 BOM' also works for Mac in this case. If we do
that, we have to make sure that it does not introduce a 'blank character' at the
beginning on other platforms. If it does, we may do that only on Mac (assuming
it works on Mac). 



> 
> 
> On Thu, Jul 9, 2009 at 2:17 PM, <avi@chromium.org> wrote:
> 
> > Reviewers: darin,
> >
> > Message:
> > What do you think of this approach? We need some way of annotating that
> > this is UTF-8. On Windows, perhaps that's assumed, but on the Mac
> > unmarked HTML is assumed to be 8859-1, which is rather bad.
> >
> > Description:
> > Mark HTML as UTF-8.
> >
> > NOT FOR COMMIT; EVAL for upstreaming
> >
> > BUG=http://crbug.com/11957
> > TEST=as in bug
> >
> >
> > Please review this at http://codereview.chromium.org/149414
> >
> > SVN Base: http://svn.webkit.org/repository/webkit/trunk/WebCore/
> >
> > Affected files:
> >  M     platform/chromium/PasteboardChromium.cpp
> >
> >
> > Index: platform/chromium/PasteboardChromium.cpp
> > ===================================================================
> > --- platform/chromium/PasteboardChromium.cpp    (revision 45659)
> > +++ platform/chromium/PasteboardChromium.cpp    (working copy)
> > @@ -71,6 +71,7 @@
> >  void Pasteboard::writeSelection(Range* selectedRange, bool
> > canSmartCopyOrDelete, Frame* frame)
> >  {
> >     String html = createMarkup(selectedRange, 0, AnnotateForInterchange);
> > +    html = String("<meta http-equiv=\"Content-Type\" content=\"text/html;
> > charset=UTF-8\">") + html;
> >     ExceptionCode ec = 0;
> >     KURL url = selectedRange->startContainer(ec)->document()->url();
> >     String plainText = frame->selectedText();
> >
> >
> >
>

[Avi (use Gerrit), 2009-07-10 14:24:24]

Let me try the BOM. I'll get back to you with the answer soon.

[Avi (use Gerrit), 2009-07-10 15:15:46]

On 2009/07/10 14:24:24, Avi wrote:
> Let me try the BOM. I'll get back to you with the answer soon.

BOM doesn't work; it makes even more of a mess.

BTW, ClipboardChromium::writeRange has very similar code. Would we need to make
a similar change there too?

(Will test on other platforms...)

[jungshik at Google, 2009-07-10 19:45:53]

On 2009/07/10 15:15:46, Avi wrote:
> On 2009/07/10 14:24:24, Avi wrote:
> > Let me try the BOM. I'll get back to you with the answer soon.
> 
> BOM doesn't work; it makes even more of a mess.

Thank you for trying it. Perhaps "<meta charset='utf-8'>" is the minimum that
works (as in html5). 

BTW, this must be one of very rare cases (if not the only case) where Windows
uses UTF-8  while Mac OS does not :-)


> 
> BTW, ClipboardChromium::writeRange has very similar code. Would we need to
make
> a similar change there too?

It seems so...

> 
> (Will test on other platforms...)

[Avi (use Gerrit), 2009-07-13 19:27:20]

On 2009/07/09 22:47:48, darin wrote:
> Did you investigate what happens on
> Windows?

Jungshik--thanks for the answer for Darin!

Darin, would this kind of code work stylewise, or should I pull out the
declaration to some kind of constant?

[darin (slow to review), 2009-07-13 19:56:55]

Stylewise, I think it is OK.  Does the shorter "<meta charset='utf-8'>" also
work?-Darin

On Mon, Jul 13, 2009 at 12:27 PM, <avi@chromium.org> wrote:

> On 2009/07/09 22:47:48, darin wrote:
>
>> Did you investigate what happens on
>> Windows?
>>
>
> Jungshik--thanks for the answer for Darin!
>
> Darin, would this kind of code work stylewise, or should I pull out the
> declaration to some kind of constant?
>
>
> http://codereview.chromium.org/149414
>

[Avi (use Gerrit), 2009-07-13 20:10:50]

On 2009/07/13 19:56:55, darin wrote:
> Does the shorter "<meta charset='utf-8'>" also
> work?

Yes it does! Nice, thanks.

BTW, estade confirms that things work fine on Linux with or without the patch. I
was going to confirm that things work OK on Windows, but would you rather just
ifdef this to be Mac-only? Which PLATFORM should I use? DARWIN is the closest I
can think of.

[jungshik at Google, 2009-07-14 00:32:19]

On 2009/07/13 20:10:50, Avi wrote:
> On 2009/07/13 19:56:55, darin wrote:
> > Does the shorter "<meta charset='utf-8'>" also
> > work?
> 
> Yes it does! Nice, thanks.
> 
> BTW, estade confirms that things work fine on Linux with or without the patch.
I
> was going to confirm that things work OK on Windows, but would you rather just
> ifdef this to be Mac-only? Which PLATFORM should I use? DARWIN is the closest
I
> can think of.

Yes, I'd rather #ifdef MAC although it's not a big deal if it works on other
platforms to prepend the content with '<meta charset="UTF-8">'. 

As to what to use for #ifdef, I'll defer to Darin :-) 

BTW, is what pinkerton mentioned in the bug (his last comment) fixed by your
Clipboard change? Well, I can test myself :-)

[Avi (use Gerrit), 2009-07-14 01:14:40]

On 2009/07/14 00:32:19, Jungshik Shin wrote:
> On 2009/07/13 20:10:50, Avi wrote:
> > On 2009/07/13 19:56:55, darin wrote:
> > > Does the shorter "<meta charset='utf-8'>" also
> > > work?
> > 
> > Yes it does! Nice, thanks.
> > 
> > BTW, estade confirms that things work fine on Linux with or without the
patch.
> I
> > was going to confirm that things work OK on Windows, but would you rather
just
> > ifdef this to be Mac-only? Which PLATFORM should I use? DARWIN is the
closest
> I
> > can think of.
> 
> Yes, I'd rather #ifdef MAC although it's not a big deal if it works on other
> platforms to prepend the content with '<meta charset="UTF-8">'. 
> 
> As to what to use for #ifdef, I'll defer to Darin :-) 
> 
> BTW, is what pinkerton mentioned in the bug (his last comment) fixed by your
> Clipboard change? Well, I can test myself :-)

By golly, it does! That's kinda awesome. I rock :P

Avi

[darin (slow to review), 2009-07-14 01:24:08]

PLATFORM(DARWIN) is what we use in WebCore within the PLATFORM(CHROMIUM)
world to select our Mac port.
-Darin


On Mon, Jul 13, 2009 at 1:10 PM, <avi@chromium.org> wrote:

> On 2009/07/13 19:56:55, darin wrote:
>
>> Does the shorter "<meta charset='utf-8'>" also
>> work?
>>
>
> Yes it does! Nice, thanks.
>
> BTW, estade confirms that things work fine on Linux with or without the
> patch. I was going to confirm that things work OK on Windows, but would
> you rather just ifdef this to be Mac-only? Which PLATFORM should I use?
> DARWIN is the closest I can think of.
>
>
> http://codereview.chromium.org/149414
>

[Avi (use Gerrit), 2009-07-14 15:15:50]

Darin:

Upstream bug at https://bugs.webkit.org/show_bug.cgi?id=27262 . Can you take a
look?

Avi

[Avi (use Gerrit), 2009-07-15 14:01:41]

Committed upstream in r45878.