Re: [webkit-dev] localStorage quota limit
Could WebKit configure the localstorage database(s) to use UTF8 text encoding for string values? On Sun, Nov 29, 2009 at 8:38 AM, William Edney wrote: > All - > > I've been discussing the localStorage quota limit over on this bug with > Jeremy Orlow: > > https://bugs.webkit.org/show_bug.cgi?id=31791 > > To recap from the discussions on that bug: > > Jeremy has implemented the localStorage quota on the latest Webkit builds. > This caused my usage of localStorage to fail, because as a JS programmer, I > assumed that 5MB meant '5 million characters' of storage. This assumption > holds true on Firefox 3.5.X+ and IE8, but fails on Webkit since it stores > things into localStorage as UTF-16. > > One option we discussed on that bug was getting the spec folks to alter the > spec in one of three ways: > > - specify the quota in terms of 'characters' (or Strings, or whatever) > thereby abstracting away the encoding problem entirely. > - specify UTF-8 so that 'MB = characters' > - specify a JS API such that the encoding could be specified. > > Jeremy wasn't too taken with any of these proposals, and in any case, they > probably need to be taken up on the W3 group defining this stuff, not here. > > In any case, as Jeremy states in Comment #5 of the bug report, "the spec's > mentioning of 5mb is really just an example". And when I filed this bug on > Mozilla's Bugzilla tracker: > > https://bugzilla.mozilla.org/show_bug.cgi?id=461684 > > another comment there points out the same thing. (Note that this bug was > originally filed to see if the Mozilla guys would raise their quota to 10MB > to match IE8 and, since they don't use double-byte encoding, I was really > asking for '10 million characters' there :-)). > > Given that, an increase from 5MB to 10MB would 'solve my immediate > problem'. And, without going back to the spec folks, I'm not sure that much > more can be done here. > > Jeremy wanted me to post to get the discussion started (and hopefully > attain some consensus :-) ), so let's discuss :-). > > Thanks in advance! > > Cheers, > > - Bill > > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev > > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] localStorage quota limit
This would probably be a performance win since it would reduce the amount of disk i/o. (Note, it doesn't mean that 5 million characters could be stored since a UTF-8 character might be multi-byte.) -Darin On Wed, Dec 2, 2009 at 9:30 AM, Michael Nordman wrote: > Could WebKit configure the localstorage database(s) to use UTF8 text > encoding for string values? > > On Sun, Nov 29, 2009 at 8:38 AM, William Edney < > bed...@technicalpursuit.com> wrote: > >> All - >> >> I've been discussing the localStorage quota limit over on this bug with >> Jeremy Orlow: >> >> https://bugs.webkit.org/show_bug.cgi?id=31791 >> >> To recap from the discussions on that bug: >> >> Jeremy has implemented the localStorage quota on the latest Webkit builds. >> This caused my usage of localStorage to fail, because as a JS programmer, I >> assumed that 5MB meant '5 million characters' of storage. This assumption >> holds true on Firefox 3.5.X+ and IE8, but fails on Webkit since it stores >> things into localStorage as UTF-16. >> >> One option we discussed on that bug was getting the spec folks to alter >> the spec in one of three ways: >> >> - specify the quota in terms of 'characters' (or Strings, or whatever) >> thereby abstracting away the encoding problem entirely. >> - specify UTF-8 so that 'MB = characters' >> - specify a JS API such that the encoding could be specified. >> >> Jeremy wasn't too taken with any of these proposals, and in any case, they >> probably need to be taken up on the W3 group defining this stuff, not here. >> >> In any case, as Jeremy states in Comment #5 of the bug report, "the spec's >> mentioning of 5mb is really just an example". And when I filed this bug on >> Mozilla's Bugzilla tracker: >> >> https://bugzilla.mozilla.org/show_bug.cgi?id=461684 >> >> another comment there points out the same thing. (Note that this bug was >> originally filed to see if the Mozilla guys would raise their quota to 10MB >> to match IE8 and, since they don't use double-byte encoding, I was really >> asking for '10 million characters' there :-)). >> >> Given that, an increase from 5MB to 10MB would 'solve my immediate >> problem'. And, without going back to the spec folks, I'm not sure that much >> more can be done here. >> >> Jeremy wanted me to post to get the discussion started (and hopefully >> attain some consensus :-) ), so let's discuss :-). >> >> Thanks in advance! >> >> Cheers, >> >> - Bill >> >> ___ >> webkit-dev mailing list >> webkit-dev@lists.webkit.org >> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev >> >> > > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev > > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] localStorage quota limit
On Dec 2, 2009, at 9:49 AM, Darin Fisher wrote: > This would probably be a performance win since it would reduce the amount of > disk i/o. > > (Note, it doesn't mean that 5 million characters could be stored since a > UTF-8 character might be multi-byte.) Currently the database can store invalid UTF-16 as well as valid UTF-16. Conversion from UTF-16 to UTF-8 might not be able to preserve invalid UTF-16 sequences. I don’t understand how the other platforms handle this. Perhaps the specification needs to be clearer on whether invalid UTF-16 is allowed. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] localStorage quota limit
+ hixie I don't know as much about encoding types as I should. How can you construct invalid UTF-16 sequences? What does Firefox or IE do with these when put into LocalStorage? One thing I just considered is that our LocalStorage implementation loads the entire LocalStorage database for an origin into memory all at once. It does this on a background thread, but it's only loaded on the first access to |window.localStorage| and we block on it finishing loading when you first try to use it. So if |alert(window.localStorage.foo);| is the first usage of LocalStorage, it (and thus the main thread) will block on the whole thing being loaded into memory. This can certainly be optimized, but I'm pointing this out because the bigger the DB is, the worse the worst case load time is. (Making this better is on my todo list, but not as near the top as I'd like.) In case you're wondring, you can mitigate this by doing 'var storage = window.localStorage;' as early as possible in your script and waiting as long as possible to actually use window.localStorage. J On Wed, Dec 2, 2009 at 10:01 AM, Darin Adler wrote: > On Dec 2, 2009, at 9:49 AM, Darin Fisher wrote: > > > This would probably be a performance win since it would reduce the amount > of disk i/o. > > > > (Note, it doesn't mean that 5 million characters could be stored since a > UTF-8 character might be multi-byte.) > > Currently the database can store invalid UTF-16 as well as valid UTF-16. > Conversion from UTF-16 to UTF-8 might not be able to preserve invalid UTF-16 > sequences. I don’t understand how the other platforms handle this. Perhaps > the specification needs to be clearer on whether invalid UTF-16 is allowed. > >-- Darin > > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Platform API for Uniscribe/ComplexTextController?
Hi Dan, On Dec 1, 2009, at 8:29 AM, Dan Bernstein wrote: > > On Nov 29, 2009, at 10:27 AM, Kevin Ollivier wrote: > >> Hi all, >> >> The wx port is starting to look at getting proper support for complex text, >> and one of the first places I started looking at was the Win and Mac port >> implementations. I noticed that the complex text code in FontWin.cpp and >> FontComplexTextMac.cpp is largely the same, passing the heavy lifting down >> to their respective complex text controllers, and I actually wondered if >> they could be merged if there were some tweaks to the APIs to make them >> compatible. >> >> That led me to wonder if we couldn't make ComplexTextController one of our >> platform classes and have USE() defines to determine the implementation. >> Then we could move the drawComplexText, floatWidthForComplexText, and >> offsetForPositionForComplexText into Font.cpp guarded by that USE() define. >> The wx port can provide the native font handles the Win/Mac controllers >> need, so it'd really be great if we could just add these into our build to >> get complex text support working without having to implement the complex >> text methods differently for each platform. >> >> BTW, I actually already did get UniscribeController compiling, actually, but >> on Windows I had to have the build script copy it to platform/graphics/wx >> because MSVC implicitly puts the current directory first on the path, which >> was causing it to pick up platform/graphics/win/FontPlatformData.h instead >> of the wx one when compiling that file. :( So that's something else that >> would need to be addressed if ports can start sharing these files. >> >> Thanks, >> >> Kevin > > Hi Kevin, > > This sounds like a generally good idea. ComplextTextController is already > using USE() macros to select between Core Text and ATSUI. I am not entirely > sure how the the *ComplexText() methods will be guarded in Font.cpp in your > proposal. Are you suggesting something like > #if USE(ATSUI) || USE(CORE_TEXT) || USE(UNISCRIBE) || … > ? I was thinking we'd create some WTF_USE_COMPLEX_TEXT_CONTROLLER so that it would be easier to opt out of using it, since a port may define / use WTF_USE_ATSUI or WTF_USE_CORE_TEXT for purposes other than supporting complex text. > There are still some differences in behavior between ComplexTextController > and UniscribeController, so you’d need to find a way to accommodate them or > eliminate them. In terms of public API, I think merging the changes shouldn't be too difficult. IIUC finalRoundingWidth() can probably just remain 0 on Win, and the "ignore writing direction" optimization on Mac can be put in the common API but just be ignored under Windows. The only thing I'm not sure about is that Mac and Win get the run's width in different ways, but I'm not quite sure why they do it differently00. Either approach would seem to work for both platforms. Mac calculates the total width when the ComplexTextController is created, while on Win to calculate it you have to call advance(run.length()) then get the value using runWidthSoFar(). The quick fix would seem to be to just have both platforms call advance(run.length()) and then use runWidthSoFar(), but I suspect that would hurt performance on Mac. Another way that might fix it would be to call advance(run.length()) in UniscribeController::UniscribeController then set m_totalWidth = m_runWidthSoFar and reset m_currentCharacter and m_runWidthSoFar to 0. Lastly, we could take the guts of advance and put it into something like a Win version of adjustGlyphsAndAdvances() that sets total width. Do you have any suggestions / preferences for how to tackle this? > I look forward to seeing a patch! Cool, I don't think this will take too long to work up. :) Thanks, Kevin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] localStorage quota limit
On Dec 2, 2009, at 10:48 AM, Jeremy Orlow wrote: > How can you construct invalid UTF-16 sequences? http://unicode.org/faq/utf_bom.html#utf16-7 -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] localStorage quota limit
IE chokes ("invalid procedure call or argument") and Firefox mangles the
data for LocalStorage (but works fine for SessionStorage).
On Wed, Dec 2, 2009 at 10:54 AM, Darin Adler wrote:
> On Dec 2, 2009, at 10:48 AM, Jeremy Orlow wrote:
>
> > How can you construct invalid UTF-16 sequences?
>
> http://unicode.org/faq/utf_bom.html#utf16-7
>
>-- Darin
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] localStorage quota limit
This is why LocalStorage quota should remain relatively small. (/me holds back urges to bitch about the LocalStorage spec.) If people want more storage space, then DB should be used, which can more efficiently accommodate large amounts of data. -Darin On Wed, Dec 2, 2009 at 10:48 AM, Jeremy Orlow wrote: > + hixie > > I don't know as much about encoding types as I should. How can you > construct invalid UTF-16 sequences? What does Firefox or IE do with these > when put into LocalStorage? > > One thing I just considered is that our LocalStorage implementation loads > the entire LocalStorage database for an origin into memory all at once. It > does this on a background thread, but it's only loaded on the first access > to |window.localStorage| and we block on it finishing loading when you first > try to use it. So if |alert(window.localStorage.foo);| is the first usage > of LocalStorage, it (and thus the main thread) will block on the whole thing > being loaded into memory. This can certainly be optimized, but I'm pointing > this out because the bigger the DB is, the worse the worst case load time > is. (Making this better is on my todo list, but not as near the top as I'd > like.) > > In case you're wondring, you can mitigate this by doing 'var storage = > window.localStorage;' as early as possible in your script and waiting as > long as possible to actually use window.localStorage. > > J > > On Wed, Dec 2, 2009 at 10:01 AM, Darin Adler wrote: > >> On Dec 2, 2009, at 9:49 AM, Darin Fisher wrote: >> >> > This would probably be a performance win since it would reduce the >> amount of disk i/o. >> > >> > (Note, it doesn't mean that 5 million characters could be stored since a >> UTF-8 character might be multi-byte.) >> >> Currently the database can store invalid UTF-16 as well as valid UTF-16. >> Conversion from UTF-16 to UTF-8 might not be able to preserve invalid UTF-16 >> sequences. I don’t understand how the other platforms handle this. Perhaps >> the specification needs to be clearer on whether invalid UTF-16 is allowed. >> >>-- Darin >> >> ___ >> webkit-dev mailing list >> webkit-dev@lists.webkit.org >> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev >> > > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] localStorage quota limit
After thinking about it a bit, I guess I feel like we should do nothing. I'm pretty against letting users set the encoding type for localStorage. That sounds like a lot of complexity for not a lot of benifit. Plus it'll cause problems when multiple web apps are in the same origin (and require different encoding) or you're using one JS library that assumes one encoding and another that assumes another. Converting to UTF 8 seems problematic. Increasing the limit encourages heavier use of LocalStorage which I'm not in favor of. Darin's right that if you want to store a lot of data and/or have more control over it, Web SQL Database is probably what you should be using. J On Wed, Dec 2, 2009 at 11:19 AM, Darin Fisher wrote: > This is why LocalStorage quota should remain relatively small. (/me holds > back urges to bitch about the LocalStorage spec.) > > If people want more storage space, then DB should be used, which can more > efficiently accommodate large amounts of data. > > -Darin > > > On Wed, Dec 2, 2009 at 10:48 AM, Jeremy Orlow wrote: > >> + hixie >> >> I don't know as much about encoding types as I should. How can you >> construct invalid UTF-16 sequences? What does Firefox or IE do with these >> when put into LocalStorage? >> >> One thing I just considered is that our LocalStorage implementation loads >> the entire LocalStorage database for an origin into memory all at once. It >> does this on a background thread, but it's only loaded on the first access >> to |window.localStorage| and we block on it finishing loading when you first >> try to use it. So if |alert(window.localStorage.foo);| is the first usage >> of LocalStorage, it (and thus the main thread) will block on the whole thing >> being loaded into memory. This can certainly be optimized, but I'm pointing >> this out because the bigger the DB is, the worse the worst case load time >> is. (Making this better is on my todo list, but not as near the top as I'd >> like.) >> >> In case you're wondring, you can mitigate this by doing 'var storage = >> window.localStorage;' as early as possible in your script and waiting as >> long as possible to actually use window.localStorage. >> >> J >> >> On Wed, Dec 2, 2009 at 10:01 AM, Darin Adler wrote: >> >>> On Dec 2, 2009, at 9:49 AM, Darin Fisher wrote: >>> >>> > This would probably be a performance win since it would reduce the >>> amount of disk i/o. >>> > >>> > (Note, it doesn't mean that 5 million characters could be stored since >>> a UTF-8 character might be multi-byte.) >>> >>> Currently the database can store invalid UTF-16 as well as valid UTF-16. >>> Conversion from UTF-16 to UTF-8 might not be able to preserve invalid UTF-16 >>> sequences. I don’t understand how the other platforms handle this. Perhaps >>> the specification needs to be clearer on whether invalid UTF-16 is allowed. >>> >>>-- Darin >>> >>> ___ >>> webkit-dev mailing list >>> webkit-dev@lists.webkit.org >>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev >>> >> >> > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] localStorage quota limit
Arguably, seems like a bug that invalid string values are let thru the door
to start with?
Since users can't effectively store invalid UTF16 character sequences in FF
or IE, is there really any downside to using UTF8 text encoding in WebKit?
@Jeremy, this isn't a matter of letting users choose the text encoding, this
is entirely an implementation detail of WebStorage.
Downsides
* The code change to get UTF8 by default in new databases, tiny.
* Migrating pre-existing databases to the new encoding. Somewhat of a
hassle. But maybe doesn't need to be done, pre-existing files could continue
to use UTF16, while newly created dbs could use UTF8 (the text encoding is
chosen at database creation time and stuck that way forever thereafter).
* Its possible that some app is already depending on the ability to store
invalid character sequences (on the iPhone say), and this would be a
breaking change for that app.
The preload everything characteristic is a separate issue altogether.
On Wed, Dec 2, 2009 at 11:15 AM, Jeremy Orlow wrote:
> IE chokes ("invalid procedure call or argument") and Firefox mangles the
> data for LocalStorage (but works fine for SessionStorage).
>
> On Wed, Dec 2, 2009 at 10:54 AM, Darin Adler wrote:
>
>> On Dec 2, 2009, at 10:48 AM, Jeremy Orlow wrote:
>>
>> > How can you construct invalid UTF-16 sequences?
>>
>> http://unicode.org/faq/utf_bom.html#utf16-7
>>
>>-- Darin
>>
>>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] localStorage quota limit
On Dec 2, 2009, at 12:06 PM, Michael Nordman wrote:
Arguably, seems like a bug that invalid string values are let thru
the door to start with?
ECMAScript Strings are essentially sequences of arbitrary 16-bit
values. Sometimes Web apps take advantage of this to use a String as a
hacky way to represent binary data. I don't think we should reject
such strings arbitrarily.
Since users can't effectively store invalid UTF16 character
sequences in FF or IE,
I tend to think this is a bug in FF/IE. Nothing in the spec gives
license to reject particular Strings.
is there really any downside to using UTF8 text encoding in WebKit?
@Jeremy, this isn't a matter of letting users choose the text
encoding, this is entirely an implementation detail of WebStorage.
I think it would be fine to use a more compact encoding
opportunistically, as long as we can still handle an arbitrary
JavaScript String. Perhaps we should use UTF-8 if and only if the
conversion succeeds, or perhaps even use Latin1 as the alternative.
Downsides
* The code change to get UTF8 by default in new databases, tiny.
* Migrating pre-existing databases to the new encoding. Somewhat of
a hassle. But maybe doesn't need to be done, pre-existing files
could continue to use UTF16, while newly created dbs could use UTF8
(the text encoding is chosen at database creation time and stuck
that way forever thereafter).
* Its possible that some app is already depending on the ability to
store invalid character sequences (on the iPhone say), and this
would be a breaking change for that app.
The preload everything characteristic is a separate issue altogether.
On Wed, Dec 2, 2009 at 11:15 AM, Jeremy Orlow
wrote:
IE chokes ("invalid procedure call or argument") and Firefox mangles
the data for LocalStorage (but works fine for SessionStorage).
On Wed, Dec 2, 2009 at 10:54 AM, Darin Adler wrote:
On Dec 2, 2009, at 10:48 AM, Jeremy Orlow wrote:
> How can you construct invalid UTF-16 sequences?
http://unicode.org/faq/utf_bom.html#utf16-7
-- Darin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] More on test flakiness
In a recent code review where I was minimizing some test flakiness, Darin said: "I wish there was a way to isolate timing-dependent tests separately from the vast majority of tests that can run at any speed. I'd prefer to not have tests that pass or fail based on the speed or load of the computer, but if we do knowingly have them it would be *so* much better if they were identified somehow." I agree, and would like to implement something to address this. Possible ways to mark a test as timing-dependent: - Put tests in a specific directory - Append a suffix to the test name - Add a function call to layoutTestController that is called explicitly for timing dependent tests >From here, the issue becomes how to use the knowledge. Some ideas: - If one of these tests fails, don't turn the bots red, turn some other color - If one of these tests fails, re-run it. If it passes the second time, consider it a normal pass - Turn bots red as normal, but with an indicator that the test is known timing dependent (if we used a suffix on the test name, I guess this would just be obvious) Thoughts? Julie ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] More on test flakiness
As Eric just said to me in person, another option is to just re-run *any* failing test twice, and only turn tree red if it fails twice. (Chromium just recently started doing this, and it has greatly improved our tree greenness). This obviously doesn't explicitly identify timing dependent tests, but it solves the bigger issues that flaky tests cause. On Wed, Dec 2, 2009 at 2:43 PM, Julie Parent > wrote: > In a recent code review where I was minimizing some test flakiness, Darin > said: > "I wish there was a way to isolate timing-dependent tests separately from > the vast majority of tests that can run at any speed. I'd prefer to not have > tests that pass or fail based on the speed or load of the computer, but if > we do knowingly have them it would be *so* much better if they were > identified somehow." > > I agree, and would like to implement something to address this. > > Possible ways to mark a test as timing-dependent: > >- Put tests in a specific directory >- Append a suffix to the test name >- Add a function call to layoutTestController that is called explicitly >for timing dependent tests > > From here, the issue becomes how to use the knowledge. Some ideas: > >- If one of these tests fails, don't turn the bots red, turn some other >color >- If one of these tests fails, re-run it. If it passes the second >time, consider it a normal pass >- Turn bots red as normal, but with an indicator that the test is known >timing dependent (if we used a suffix on the test name, I guess this would >just be obvious) > > Thoughts? > > Julie > > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] More on test flakiness
On Wed, Dec 2, 2009 at 2:51 PM, Julie Parent > wrote: > As Eric just said to me in person, another option is to just re-run *any* > failing test twice, and only turn tree red if it fails twice. (Chromium > just recently started doing this, and it has greatly improved our tree > greenness). Do we still note the flaky tests so we can try to fix them? I worry that some flakiness is actually due to real bugs (that cause product flakiness) as opposed to test bugs, though I'm not sure that seeing tests fail flakily actually helps us address that... PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] More on test flakiness
On Wed, Dec 2, 2009 at 3:02 PM, Peter Kasting wrote: > On Wed, Dec 2, 2009 at 2:51 PM, Julie Parent > wrote: >> >> As Eric just said to me in person, another option is to just re-run *any* >> failing test twice, and only turn tree red if it fails twice. (Chromium >> just recently started doing this, and it has greatly improved our tree >> greenness). > > Do we still note the flaky tests so we can try to fix them? I worry that > some flakiness is actually due to real bugs (that cause product flakiness) > as opposed to test bugs, though I'm not sure that seeing tests fail flakily > actually helps us address that... Yes, we still track flaky tests. -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Switch statement indentation
The style script flagged an issue in my code yesterday for an issue I didn't even know existed. How do you indent case clauses for a switch statement? The WebKit style states that case clauses have the same indentation as their switch. I HATE that style. And I had no idea that was the WebKit style. I use the "indent the case" style and have never had anyone flag it in the past. Without getting into style religion, I was looking at the code and it seems that there are many more uses of the "indent the case" style than the "correct" style. Maybe we could change the style rule in the interest of changing fewer files (and because I think it generally reads better)? I'm fine with changing my code to match the style. But the style script is going to be kicking out a lot of these errors and I think we should make sure we want to go down this road before that happens. - ~Chris cmar...@apple.com ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Switch statement indentation
On 02.12.2009, at 15:25, Chris Marrin wrote:
Maybe we could change the style rule in the interest of changing
fewer files (and because I think it generally reads better)?
I support changing or dropping this rule. Because of this rule, there
is no good way to format cases that need braces, such as:
switch (i) {
case 1: {
String a("a");
break;
}
case 2: {
String b("b");
break;
}
}
The downside is that some code can get indented too far, which is
particularly unfortunate for large switches. But I'm not convinced
that having a standard for this improves consistency of the code in
any meaningful way (*), perhaps this should be decided on a case by
case basis.
(*) meaningful == helps to read the code, or to search, or to process
with scripts, or maybe even to copy/paste.
- WBR, Alexey Proskuryakov
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Switch statement indentation
On Dec 2, 2009, at 4:47 PM, Alexey Proskuryakov wrote:
On 02.12.2009, at 15:25, Chris Marrin wrote:
Maybe we could change the style rule in the interest of changing
fewer files (and because I think it generally reads better)?
I support changing or dropping this rule. Because of this rule,
there is no good way to format cases that need braces, such as:
switch (i) {
case 1: {
String a("a");
break;
}
case 2: {
String b("b");
break;
}
}
The downside is that some code can get indented too far, which is
particularly unfortunate for large switches. But I'm not convinced
that having a standard for this improves consistency of the code in
any meaningful way (*), perhaps this should be decided on a case by
case basis.
The "indented too far" problem can be solved by sticking really big
switches in their own function. I think this is better style anyway.
I've always found huge switches in the middle of a long function to be
very confusing.
-
~Chris
cmar...@apple.com
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] localStorage quota limit
On Wed, 2 Dec 2009, Michael Nordman wrote: > > Arguably, seems like a bug that invalid string values are let thru the > door to start with? Yeah, I should make the spec through SYNTAX_ERR if there are any unpaired surrogates, the same way WebSocket does. I'll file a bug. -- Ian Hickson U+1047E)\._.,--,'``.fL http://ln.hixie.ch/ U+263A/, _.. \ _\ ;`._ ,. Things that are impossible just take longer. `._.-(,_..'--(,_..'`-.;.' ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Switch statement indentation
> The downside is that some code can get indented too far, which is > particularly unfortunate for large switches. We could issue a fuzzy declaration such as, "Indent case blocks, except in situations where an unreasonable amount of code would end up so-indented, causing readability problems". Two examples of situations where indenting case blocks would cause readability problems are the JavaScriptCore interpreter and, to a lesser extent, the JavaScriptCore JIT. All of Interpreter.cpp would suddenly be indented by an extra 4 spaces, sucking up valuable horizontal real estate. Geoff ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Switch statement indentation
On Dec 2, 2009, at 7:56 PM, Geoffrey Garen wrote: The downside is that some code can get indented too far, which is particularly unfortunate for large switches. We could issue a fuzzy declaration such as, "Indent case blocks, except in situations where an unreasonable amount of code would end up so-indented, causing readability problems". Two examples of situations where indenting case blocks would cause readability problems are the JavaScriptCore interpreter and, to a lesser extent, the JavaScriptCore JIT. All of Interpreter.cpp would suddenly be indented by an extra 4 spaces, sucking up valuable horizontal real estate. I believe one rule that could work is something like this: - Indent case labels inside a switch two spaces. - Indent actual statements inside a switch four spaces. - In the case where a case label is followed by a block, include the open brace on the same line as the case label and indent the matching close brace only two spaces (but still 4 spaces for the contained statements). That would not cause any code to be excessively indented, but would avoid some of the downsides of not indenting at all mentioned by Chris. I would rather have a clear rule that makes sense in a variety of situations than a fuzzy guideline. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] localStorage quota limit
What about Maciej's comment. JS strings are often use to store binary values. Obviously, if people stick to octets, then it should be fine, but perhaps some folks leverage all 16 bits? -Darin On Wed, Dec 2, 2009 at 5:03 PM, Ian Hickson wrote: > On Wed, 2 Dec 2009, Michael Nordman wrote: > > > > Arguably, seems like a bug that invalid string values are let thru the > > door to start with? > > Yeah, I should make the spec through SYNTAX_ERR if there are any unpaired > surrogates, the same way WebSocket does. I'll file a bug. > > -- > Ian Hickson U+1047E)\._.,--,'``.fL > http://ln.hixie.ch/ U+263A/, _.. \ _\ ;`._ ,. > Things that are impossible just take longer. `._.-(,_..'--(,_..'`-.;.' > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] localStorage quota limit
On Dec 2, 2009, at 8:14 PM, Darin Fisher wrote: What about Maciej's comment. JS strings are often use to store binary values. Obviously, if people stick to octets, then it should be fine, but perhaps some folks leverage all 16 bits? I think some people do use JavaScript strings this way, though not necessarily with LocalStorage. This kind of use will probably become obsolete when we add a proper way to store binary data from the platform. Most Web-related APIs are fully accepting of JavaScript strings that are not proper UTF-16. I don't see a strong reason to make LocalStorage an exception. It does make sense for WebSocket to be an exception, since in that case charset transcoding is required by the protocol, and since it is desirable in that case to prevent any funny business that may trip up the server.. Also, looking at UTF-16 more closely, it seems like all UTF-16 can be transcoded to UTF-8 and round-tripped if one is willing to allow technically invalid UTF-8 that encodes unpaired characters in the surrogate range as if they were characters. It's not clear to me why Firefox or IE choose to reject instead of doing this. This also removes my original objection to storing strings as UTF-8. Regards, Maciej -Darin On Wed, Dec 2, 2009 at 5:03 PM, Ian Hickson wrote: On Wed, 2 Dec 2009, Michael Nordman wrote: > > Arguably, seems like a bug that invalid string values are let thru the > door to start with? Yeah, I should make the spec through SYNTAX_ERR if there are any unpaired surrogates, the same way WebSocket does. I'll file a bug. -- Ian Hickson U+1047E) \._.,--,'``.fL http://ln.hixie.ch/ U+263A/, _.. \ _ \ ;`._ ,. Things that are impossible just take longer. `._.-(,_..'-- (,_..'`-.;.' ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Switch statement indentation
On Wed, Dec 2, 2009 at 8:05 PM, Maciej Stachowiak wrote: > I believe one rule that could work is something like this: > > - Indent case labels inside a switch two spaces. > - Indent actual statements inside a switch four spaces. > - In the case where a case label is followed by a block, include the open > brace on the same line as the case label and indent the matching close brace > only two spaces (but still 4 spaces for the contained statements). > This is precisely the rule that the Google C++ Style Guide uses ( http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Loops_and_Switch_Statements). I think it works well. I support it. PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Proposing style guide change regarding braces on conditional arms
This is a followup to my thread yesterday regarding consistent enforcement
of the style guide. Like Yong Li, I find the current rule about braces on
conditional arms to be suboptimal. The current rule is that one-line arms
must not have braces. This leads to strange constructions like:
if (foo) {
a;
b;
c;
// etc., very long body
} else
x;
...or perhaps:
if (foo)
a;
else if (bar) {
b;
c;
} else if (baz)
d;
else if (qux) {
e;
f;
}
I find this tricky to read and error-prone. I propose that the rule be
modified to be:
* When all arms of a conditional or loop are one physical line, do not use
braces. If any arms are more than one physical line (even if they are one
logical line), use braces on all arms.
In most places this will not differ from the existing code, so it will not
"cause the whole codebase to become invalid"; but it prevents cases where
the inconsistency leads (IMO) to lower readability/safety. (As a bonus for
Chromium developers, it's compatible with the Google style guide too,
although it goes further than that guide in order to make the correct style
explicit in all cases.)
PK
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] localStorage quota limit
On Wed, Dec 2, 2009 at 8:44 PM, Maciej Stachowiak wrote: > > On Dec 2, 2009, at 8:14 PM, Darin Fisher wrote: > > What about Maciej's comment. JS strings are often use to store binary > values. Obviously, if people stick to octets, then it should be fine, but > perhaps some folks leverage all 16 bits? > > > I think some people do use JavaScript strings this way, though not > necessarily with LocalStorage. This kind of use will probably become > obsolete when we add a proper way to store binary data from the platform. > > Most Web-related APIs are fully accepting of JavaScript strings that are > not proper UTF-16. I don't see a strong reason to make LocalStorage an > exception. It does make sense for WebSocket to be an exception, since in > that case charset transcoding is required by the protocol, and since it is > desirable in that case to prevent any funny business that may trip up the > server.. > > Also, looking at UTF-16 more closely, it seems like all UTF-16 can be > transcoded to UTF-8 and round-tripped if one is willing to allow technically > invalid UTF-8 that encodes unpaired characters in the surrogate range as if > they were characters. It's not clear to me why Firefox or IE choose to > reject instead of doing this. This also removes my original objection to > storing strings as UTF-8. > > I think it is typical for UTF-16 to UTF-8 conversion to involve the intermediate step of forming a Unicode code point. If that cannot be done, then conversion fails. This may actually be a security thing. If something expects UTF-8, it is safer to ensure that it gets valid UTF-8 (even if that involves loss of information). -Darin > Regards, > Maciej > > > -Darin > > On Wed, Dec 2, 2009 at 5:03 PM, Ian Hickson wrote: > >> On Wed, 2 Dec 2009, Michael Nordman wrote: >> > >> > Arguably, seems like a bug that invalid string values are let thru the >> > door to start with? >> >> Yeah, I should make the spec through SYNTAX_ERR if there are any unpaired >> surrogates, the same way WebSocket does. I'll file a bug. >> >> -- >> Ian Hickson U+1047E)\._.,--,'``.fL >> http://ln.hixie.ch/ U+263A/, _.. \ _\ ;`._ ,. >> Things that are impossible just take longer. `._.-(,_..'--(,_..'`-.;.' >> ___ >> webkit-dev mailing list >> webkit-dev@lists.webkit.org >> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev >> > > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev > > > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] localStorage quota limit
On Wed, 2 Dec 2009, Maciej Stachowiak wrote: > On Dec 2, 2009, at 8:14 PM, Darin Fisher wrote: > > What about Maciej's comment. JS strings are often use to store binary > > values. Obviously, if people stick to octets, then it should be fine, > > but perhaps some folks leverage all 16 bits? > > I think some people do use JavaScript strings this way, though not > necessarily with LocalStorage. This kind of use will probably become > obsolete when we add a proper way to store binary data from the > platform. > > Most Web-related APIs are fully accepting of JavaScript strings that are > not proper UTF-16. I don't see a strong reason to make LocalStorage an > exception. I recommend raising these points on: http://www.w3.org/Bugs/Public/show_bug.cgi?id=8425 -- Ian Hickson U+1047E)\._.,--,'``.fL http://ln.hixie.ch/ U+263A/, _.. \ _\ ;`._ ,. Things that are impossible just take longer. `._.-(,_..'--(,_..'`-.;.' ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Proposing style guide change regarding braces on conditional arms
On 2009-12-02, at 21:00, Peter Kasting wrote: > I find this tricky to read and error-prone. I propose that the rule be > modified to be: > > * When all arms of a conditional or loop are one physical line, do not use > braces. If any arms are more than one physical line (even if they are one > logical line), use braces on all arms. I do not agree that this would be an improvement. > In most places this will not differ from the existing code, so it will not > "cause the whole codebase to become invalid"; I glanced briefly at three files: RenderObject.cpp, Element.cpp and Node.cpp. In these three files I counted over two dozen places that would require modification to conform with this new rule. That's by no means a majority of the relevant statements in these files, but it's not a small number either. - Mark smime.p7s Description: S/MIME cryptographic signature ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Proposing style guide change regarding braces on conditional arms
On Wed, Dec 2, 2009 at 9:19 PM, Mark Rowe wrote: > On 2009-12-02, at 21:00, Peter Kasting wrote: > > > I find this tricky to read and error-prone. I propose that the rule be > modified to be: > > > > * When all arms of a conditional or loop are one physical line, do not > use braces. If any arms are more than one physical line (even if they are > one logical line), use braces on all arms. > > I do not agree that this would be an improvement. > Are you satisfied with the existing rule, then? If so, you would be the first developer I have asked who is. Admittedly my sample size is quite small, but all those I've asked would prefer either a rule like my proposal or a rule that says "braces always", which I find to be consistent and easy to remember but somewhat verbose (and with a greater impact on the existing code, for what that's worth). > In most places this will not differ from the existing code, so it will not > "cause the whole codebase to become invalid"; > > I glanced briefly at three files: RenderObject.cpp, Element.cpp and > Node.cpp. In these three files I counted over two dozen places that would > require modification to conform with this new rule. That's by no means a > majority of the relevant statements in these files, but it's not a small > number either. I stand by my statement. In Element.cpp, 9 conditionals violate this out of roughly 200 conditionals and loops. One of these violates the existing style guide too. "Not a majority" is a significant understatement. Especially when compared to our namespace change that affects nearly every header, or the proposed case indenting change that affects every switch statement. That said, I don't think "doesn't change a large percentage of the code" is a significant argument for a rule, it mostly speaks to how people wouldn't need to massively shift their thinking when writing code because the rules have changed. PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Towards a More Convenient IWebUIDelegate
I recently explored the IWebUIDelegate interface to customize the context menu in WebKit. In Cocoa, using the WebUIDelegate is very convenient -- I simply provide an implementation for the one or two methods I wish to customize, and leave the others alone. I was proudly showing a coworker how cool and simple it was going to be to customize the context menu in our Windows build by implementing a delegate class to control this, when to my horror I realized that IWebUIDelegate (which is the equivalent object on the Windows side) is implemented as a set of pure virtual methods, all of which must be stubbed out to successfully compile. To add insult to injury, all of these interface classes also require each concrete implementation to implement a stub QueryInterface, AddRef, and RemoveRef method. This seems likely to result in a huge amount of wasted boilerplate code, where a Windows developer must create a page of stubbed implementations just to turn off (or otherwise modify) the context menu. This same anti-pattern can be found in the other I...Delegate objects as well (which also require the same boilerplate QueryInterface, AddRef, RemoveRef, etc.) For my own purposes, I created a concrete class "WebUIDelegate" that stubs all methods as E_NOTIMPL and provides the boilerplate QueryInterface, AddRef, etc.. I subclass from *this* class as my delegate, which allows me to simply write one method that does the menu customization. Couldn't the Windows version of WebKit either stub the IWebUIDelegate with the requisite E_NOTIMPL stubs, or perhaps ship with a set of pre-generated stub classes like the one I just created, to avoid this kind of drudgery? Thanks, -Brent ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Proposing style guide change regarding braces on conditional arms
On Dec 2, 2009, at 9:46 PM, Peter Kasting wrote: > On Wed, Dec 2, 2009 at 9:19 PM, Mark Rowe wrote: > On 2009-12-02, at 21:00, Peter Kasting wrote: > > > I find this tricky to read and error-prone. I propose that the rule be > > modified to be: > > > > * When all arms of a conditional or loop are one physical line, do not use > > braces. If any arms are more than one physical line (even if they are one > > logical line), use braces on all arms. > > I do not agree that this would be an improvement. > > Are you satisfied with the existing rule, then? If so, you would be the > first developer I have asked who is. I am satisfied with the existing rule.___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Proposing style guide change regarding braces on conditional arms
On 2009-12-02, at 21:46, Peter Kasting wrote: > On Wed, Dec 2, 2009 at 9:19 PM, Mark Rowe wrote: > On 2009-12-02, at 21:00, Peter Kasting wrote: > > > I find this tricky to read and error-prone. I propose that the rule be > > modified to be: > > > > * When all arms of a conditional or loop are one physical line, do not use > > braces. If any arms are more than one physical line (even if they are one > > logical line), use braces on all arms. > > I do not agree that this would be an improvement. > > Are you satisfied with the existing rule, then? I am satisfied with the existing rule, and I have seen nothing to support the suggestion that a change to the rule would provide additional benefits. > > In most places this will not differ from the existing code, so it will not > > "cause the whole codebase to become invalid"; > > I glanced briefly at three files: RenderObject.cpp, Element.cpp and Node.cpp. > In these three files I counted over two dozen places that would require > modification to conform with this new rule. That's by no means a majority of > the relevant statements in these files, but it's not a small number either. > > I stand by my statement. I was not putting this forward as my reasoning for not changing the style, merely as a data point. - Mark smime.p7s Description: S/MIME cryptographic signature ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] localStorage quota limit
On Dec 2, 2009, at 9:07 PM, Darin Fisher wrote: On Wed, Dec 2, 2009 at 8:44 PM, Maciej Stachowiak wrote: On Dec 2, 2009, at 8:14 PM, Darin Fisher wrote: What about Maciej's comment. JS strings are often use to store binary values. Obviously, if people stick to octets, then it should be fine, but perhaps some folks leverage all 16 bits? I think some people do use JavaScript strings this way, though not necessarily with LocalStorage. This kind of use will probably become obsolete when we add a proper way to store binary data from the platform. Most Web-related APIs are fully accepting of JavaScript strings that are not proper UTF-16. I don't see a strong reason to make LocalStorage an exception. It does make sense for WebSocket to be an exception, since in that case charset transcoding is required by the protocol, and since it is desirable in that case to prevent any funny business that may trip up the server.. Also, looking at UTF-16 more closely, it seems like all UTF-16 can be transcoded to UTF-8 and round-tripped if one is willing to allow technically invalid UTF-8 that encodes unpaired characters in the surrogate range as if they were characters. It's not clear to me why Firefox or IE choose to reject instead of doing this. This also removes my original objection to storing strings as UTF-8. I think it is typical for UTF-16 to UTF-8 conversion to involve the intermediate step of forming a Unicode code point. If that cannot be done, then conversion fails. This may actually be a security thing. If something expects UTF-8, it is safer to ensure that it gets valid UTF-8 (even if that involves loss of information). These security considerations seem important for WebSocket where the protocol uses UTF-8 per spec, but not for the internal storage representation of JavaScript strings in LocalStorage (where observable input and output are both possibly-invalid UTF-16). Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Towards a More Convenient IWebUIDelegate
On Dec 2, 2009, at 9:53 PM, Brent Fulgham wrote: To add insult to injury, all of these interface classes also require each concrete implementation to implement a stub QueryInterface, AddRef, and RemoveRef method. Yes, this is one of the downsides of COM. Some COM developers use ATL (or similar) to avoid boilerplate implementations of AddRef/Release/ QueryInterface. For my own purposes, I created a concrete class "WebUIDelegate" that stubs all methods as E_NOTIMPL and provides the boilerplate QueryInterface, AddRef, etc.. I subclass from *this* class as my delegate, which allows me to simply write one method that does the menu customization. Sounds similar to what we do. Couldn't the Windows version of WebKit either stub the IWebUIDelegate with the requisite E_NOTIMPL stubs, or perhaps ship with a set of pre-generated stub classes like the one I just created, to avoid this kind of drudgery? COM interfaces are pure virtual, so there is no implementation of IWebUIDelegate in WebKit. We could provide a concrete CLSID_WebUIDelegate that was instantiatable via (our version of) CoCreateInstance but you wouldn't be able to subclass it. We don't export any of the WebKit COM API directly through DLL exports. All of these conventions were critically important when the COM API was remotable via DCOM in order for Drosera (Web Inspector in its previous iteration) to work out-of-process. Some of this isn't as important now, but we'd like to maintain compatibility for existing clients. -steve ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] localStorage quota limit
On Wed, Dec 2, 2009 at 10:20 PM, Maciej Stachowiak wrote: > > On Dec 2, 2009, at 9:07 PM, Darin Fisher wrote: > > On Wed, Dec 2, 2009 at 8:44 PM, Maciej Stachowiak wrote: > >> >> On Dec 2, 2009, at 8:14 PM, Darin Fisher wrote: >> >> What about Maciej's comment. JS strings are often use to store binary >> values. Obviously, if people stick to octets, then it should be fine, but >> perhaps some folks leverage all 16 bits? >> >> >> I think some people do use JavaScript strings this way, though not >> necessarily with LocalStorage. This kind of use will probably become >> obsolete when we add a proper way to store binary data from the platform. >> >> Most Web-related APIs are fully accepting of JavaScript strings that are >> not proper UTF-16. I don't see a strong reason to make LocalStorage an >> exception. It does make sense for WebSocket to be an exception, since in >> that case charset transcoding is required by the protocol, and since it is >> desirable in that case to prevent any funny business that may trip up the >> server.. >> >> Also, looking at UTF-16 more closely, it seems like all UTF-16 can be >> transcoded to UTF-8 and round-tripped if one is willing to allow technically >> invalid UTF-8 that encodes unpaired characters in the surrogate range as if >> they were characters. It's not clear to me why Firefox or IE choose to >> reject instead of doing this. This also removes my original objection to >> storing strings as UTF-8. >> >> > I think it is typical for UTF-16 to UTF-8 conversion to involve the > intermediate step of forming a Unicode code point. If that cannot be done, > then conversion fails. This may actually be a security thing. If something > expects UTF-8, it is safer to ensure that it gets valid UTF-8 (even if that > involves loss of information). > > > These security considerations seem important for WebSocket where the > protocol uses UTF-8 per spec, but not for the internal storage > representation of JavaScript strings in LocalStorage (where observable input > and output are both possibly-invalid UTF-16). > > Regards, > Maciej > > Agreed. I was responding to your statement: "It's not clear to me why Firefox or IE choose to reject instead of doing this." It seems likely to me that neither Firefox nor IE made a concerted choice to treat bad UTF-16 this way. It is probably just a consequence of using the default UTF-16 to UTF-8 converter, which likely behaves as I described. -Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] localStorage quota limit
On Dec 2, 2009, at 10:51 PM, Darin Fisher wrote: Agreed. I was responding to your statement: "It's not clear to me why Firefox or IE choose to reject instead of doing this." It seems likely to me that neither Firefox nor IE made a concerted choice to treat bad UTF-16 this way. It is probably just a consequence of using the default UTF-16 to UTF-8 converter, which likely behaves as I described. That makes sense. - Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Towards a More Convenient IWebUIDelegate
On Dec 2, 2009, at 10:47 PM, Steve Falkenburg wrote: On Dec 2, 2009, at 9:53 PM, Brent Fulgham wrote: To add insult to injury, all of these interface classes also require each concrete implementation to implement a stub QueryInterface, AddRef, and RemoveRef method. Yes, this is one of the downsides of COM. Some COM developers use ATL (or similar) to avoid boilerplate implementations of AddRef/ Release/QueryInterface. For my own purposes, I created a concrete class "WebUIDelegate" that stubs all methods as E_NOTIMPL and provides the boilerplate QueryInterface, AddRef, etc.. I subclass from *this* class as my delegate, which allows me to simply write one method that does the menu customization. Sounds similar to what we do. Couldn't the Windows version of WebKit either stub the IWebUIDelegate with the requisite E_NOTIMPL stubs, or perhaps ship with a set of pre-generated stub classes like the one I just created, to avoid this kind of drudgery? COM interfaces are pure virtual, so there is no implementation of IWebUIDelegate in WebKit. We could provide a concrete CLSID_WebUIDelegate that was instantiatable via (our version of) CoCreateInstance but you wouldn't be able to subclass it. We don't export any of the WebKit COM API directly through DLL exports. All of these conventions were critically important when the COM API was remotable via DCOM in order for Drosera (Web Inspector in its previous iteration) to work out-of-process. Some of this isn't as important now, but we'd like to maintain compatibility for existing clients. I think we could provide a DefaultWebUIDelegate which has a default implementation of all the IWebUIDelegate methods and which clients can subclass. That would not be available via DCOM, but it's not a critical loss of functionality since you can still write your own IWebUIDelegate from scratch in that case. Likewise for the other delegates. egards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Proposing style guide change regarding braces on conditional arms
Are you satisfied with the existing rule, then? If so, you would be the first developer I have asked who is. I am satisfied with the existing rule. I'm frustrated by this existing rule. I would be very happy if the rule was changed to "We may omit braces for one-line control clauses." -- TAMURA Kent Software Engineer, Google ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Not Implemented "selectAll"
I noticed that the GTK and some other ports provide a "selectAll" method as part of the default context menu, but that this is disabled in the various Apple builds. Is this just a style choice? Or is there some reason this works in GTK but not Cocoa or Windows? I notice that the IWebFrame interface provides a stub for this through the IWebDocumentText interface, which I was happily connecting to when I realized that it dead-ends in a "notImplemented" call. Is this just something no one has completed implementing? Or was it discarded at some point? Since the Windows CTRL-A keystroke (and Mac Command-A keystroke) does a select all action, I'm a bit surprised that this is not accessible in the right-click menu. Thanks, -Brent ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Proposing style guide change regarding braces on conditional arms
I used to dislike everything about this rule; my original preference
before we codified our style guidelines was to always put braces
around the body of a control statement, even if it is only one line.
Over time, I've gotten used to it and I find this:
if (condA)
doA();
more pleasant to read than this:
if (condA) {
doA();
}
Even for if/else statements where only one branch is single-line, I'm
ok with the official style rule, although I can't say I actively like
it. So these two examples look ok to me:
if (condA)
doA();
else {
doB1();
doB2();
}
if (condA) {
doA1();
doA2();
} else
doB();
However, when there is a lengthy chain of if ... else if ... else
conditionals and a few of the blocks in the middle happens to be only
a single line each, then I tend to find the code harder to write, read
and modify. This pattern often comes up in the parseMappedAttribute()
implementations of Element subclasses.
Regardless of the specific outcome, I would strongly prefer to have a
consistent rule for this than to make it a matter of taste. One
possible conservative modification to the rule is that if you have a
multi-block if ... else if ... else chain, then if even one body has
braces, all should. I think that would tend to reduce rather than
increase visual noise, as all the "else" keywords would line up where
right now they look ragged.
I would also not object to bigger changes in the rule if that were
truly the community consensus, but personally I would set the barrier
pretty high for a rule change that would implicate large chunks of
existing code, and I think the benefit is much lower for if/else
statements with only two clauses.
Regards,
Maciej
On Dec 2, 2009, at 9:00 PM, Peter Kasting wrote:
This is a followup to my thread yesterday regarding consistent
enforcement of the style guide. Like Yong Li, I find the current
rule about braces on conditional arms to be suboptimal. The current
rule is that one-line arms must not have braces. This leads to
strange constructions like:
if (foo) {
a;
b;
c;
// etc., very long body
} else
x;
...or perhaps:
if (foo)
a;
else if (bar) {
b;
c;
} else if (baz)
d;
else if (qux) {
e;
f;
}
I find this tricky to read and error-prone. I propose that the rule
be modified to be:
* When all arms of a conditional or loop are one physical line, do
not use braces. If any arms are more than one physical line (even
if they are one logical line), use braces on all arms.
In most places this will not differ from the existing code, so it
will not "cause the whole codebase to become invalid"; but it
prevents cases where the inconsistency leads (IMO) to lower
readability/safety. (As a bonus for Chromium developers, it's
compatible with the Google style guide too, although it goes further
than that guide in order to make the correct style explicit in all
cases.)
PK
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
