RFR: 8211958: Broken links in java.desktop files
Most of the broken links were fixed already by the JDK-8225368 and JDK-8214817 This change fix just a few. Also two cleanups are applied: - The `../../java/awt/doc-files/` in some cases simplified to `doc-files/` - The html links to the java classes via `` replaced by the `{@link }` - Commit messages: - Initial fix Changes: https://git.openjdk.java.net/jdk/pull/998/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=998&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8211958 Stats: 43 lines in 9 files changed: 0 ins; 4 del; 39 mod Patch: https://git.openjdk.java.net/jdk/pull/998.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/998/head:pull/998 PR: https://git.openjdk.java.net/jdk/pull/998
Re: RFR: 8211958: Broken links in java.desktop files
On Mon, 2 Nov 2020 09:04:39 GMT, Sergey Bylokhov wrote: > Most of the broken links were fixed already by the JDK-8225368 and JDK-8214817 > This change fix just a few. > > Also two cleanups are applied: > - The `../../java/awt/doc-files/` in some cases simplified to `doc-files/` > - The html links to the java classes via `` replaced by the `{@link > }` src/java.desktop/share/classes/java/awt/doc-files/DesktopProperties.html line 101: > 99: > 100: The return value is a > 101: Map of Broken link to Map. src/java.desktop/share/classes/javax/print/DocFlavor.java line 159: > 157: * common flavors, the pre-defined *HOST {@code DocFlavors} may be used. > 158: * > 159: * See character Broken link to the java.base module - PR: https://git.openjdk.java.net/jdk/pull/998
Re: RFR: 8211958: Broken links in java.desktop files
On Mon, 2 Nov 2020 09:04:39 GMT, Sergey Bylokhov wrote: > Most of the broken links were fixed already by the JDK-8225368 and JDK-8214817 > This change fix just a few. > > Also two cleanups are applied: > - The `../../java/awt/doc-files/` in some cases simplified to `doc-files/` > - The html links to the java classes via `` replaced by the `{@link > }` Marked as reviewed by aivanov (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/998
RFR: 8255226: (tz) Upgrade time-zone data to tzdata2020d
Hi Guys, Please review the integration of tzdata2020d to JDK. Details regarding the change can be viewed at - https://mm.icann.org/pipermail/tz-announce/2020-October/62.html Bug: https://bugs.openjdk.java.net/browse/JDK-8255226 TestZoneInfo310.java test failure is addressed along with it. The last rule affects "Asia/Gaza" and "Asia/Hebron" and therefore excluded from the test. Regression Tests pass along with JCK. Please let me know if the changes are good to push. Thanks, Kiran - Commit messages: - 8255226: (tz) Upgrade time-zone data to tzdata2020d Changes: https://git.openjdk.java.net/jdk/pull/1012/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1012&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8255226 Stats: 54 lines in 4 files changed: 34 ins; 2 del; 18 mod Patch: https://git.openjdk.java.net/jdk/pull/1012.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1012/head:pull/1012 PR: https://git.openjdk.java.net/jdk/pull/1012
Integrated: 8255671: Bidi.reorderVisually has misleading exception messages
On Fri, 30 Oct 2020 22:23:30 GMT, Naoto Sato wrote: > Hi, > > Please review this simple message fix that follows JDK-8255242. > > Naoto This pull request has now been integrated. Changeset: 6dac8d27 Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/6dac8d27 Stats: 35 lines in 2 files changed: 32 ins; 0 del; 3 mod 8255671: Bidi.reorderVisually has misleading exception messages Reviewed-by: joehw - PR: https://git.openjdk.java.net/jdk/pull/973
Re: RFR: 8255226: (tz) Upgrade time-zone data to tzdata2020d
On Mon, 2 Nov 2020 16:29:07 GMT, Kiran Sidhartha Ravikumar wrote: > Hi Guys, > > Please review the integration of tzdata2020d to JDK. > > Details regarding the change can be viewed at - > https://mm.icann.org/pipermail/tz-announce/2020-October/62.html > Bug: https://bugs.openjdk.java.net/browse/JDK-8255226 > > TestZoneInfo310.java test failure is addressed along with it. The last rule > affects "Asia/Gaza" and "Asia/Hebron" and therefore excluded from the test. > > Regression Tests pass along with JCK. > > Please let me know if the changes are good to push. > > Thanks, > Kiran The test case needs copyright year change to 2020. test/jdk/sun/util/calendar/zi/TestZoneInfo310.java line 201: > 199: zid.equals("Iran") || // last rule mismatch > 200: zid.equals("Asia/Gaza") || // last rule mismatch > 201: zid.equals("Asia/Hebron")) { // last rule mismatch Can you explain why these zones are failing? The criteria for excluding failing tests here is that the zone has negative dst and last rules beyond 2037, and I don't think those newly excluded zones suffice those. - Changes requested by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1012
Re: RFR: 8247781: Day periods support [v3]
On Fri, 30 Oct 2020 11:06:59 GMT, Stephen Colebourne wrote: >> Naoto Sato has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fixed exception messages. > > src/java.base/share/classes/java/time/format/Parsed.java line 497: > >> 495: AMPM_OF_DAY.checkValidValue(ap); >> 496: } >> 497: updateCheckDayPeriodConflict(AMPM_OF_DAY, midpoint >> / 720); > > No need to put `AMPM_OF_DAY` back in here because you've already resolved it > to `HOUR_OF_DAY` and `MINUTE_OF_HOUR`. There probably does need to be > validation to check that the day period agrees with the AM/PM value. Line can still be removed AFAICT - PR: https://git.openjdk.java.net/jdk/pull/938
Re: RFR: 8247781: Day periods support [v3]
On Fri, 30 Oct 2020 22:03:08 GMT, Naoto Sato wrote: >> Hi, >> >> Please review the changes for the subject issue. This is to enhance the >> java.time package to support day periods, such as "in the morning", defined >> in CLDR. It will add a new pattern character 'B' and its supporting builder >> method. The motivation and its spec are in this CSR: >> >> https://bugs.openjdk.java.net/browse/JDK-8254629 >> >> Naoto > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed exception messages. test/jdk/java/time/test/java/time/format/TestDateTimeFormatterBuilder.java line 656: > 654: {"h B", "11 at night", 23}, > 655: {"h B", "3 at night", 3}, > 656: {"h B", "11 in the morning", 11}, Need tests for "51 in the morning" (which should parse in LENIENT as "3 in the morning" plus 2 days, see how HOUR_OF_DAY=51 works in general. Similar issue with HOUR_OF_AMPM=3 and AMPM_OF_DAY=4. - PR: https://git.openjdk.java.net/jdk/pull/938
Re: RFR: 8247781: Day periods support [v3]
On Fri, 30 Oct 2020 21:30:50 GMT, Naoto Sato wrote: >> src/java.base/share/classes/java/time/format/Parsed.java line 472: >> >>> 470: } >>> 471: if (dayPeriod != null) { >>> 472: if (fieldValues.containsKey(HOUR_OF_DAY)) { >> >> Are we certain that the CLDR data does not contain day periods based on >> minutes as well as hours? This logic does not check against MINUTE_OF_HOUR >> for example. The logic also conflicts with the spec Javadoc that says >> MINUTE_OF_HOUR is validated. > > MINUTE_OF_HOUR without HOUR_OF_DAY/HOUR_OF_AMPM may not make any sense, so it > is only validated in updateCheckDayPeriodConflict. But can you get a CLDR rule that says "at night" before 05:30 and "In the morning" from 05:30 onwards? If you can then I don't think it is handled, because HOUR_OF_DAY and MINUTE_OF_HOUR are not used together when checking against DayPeriod. >> src/java.base/share/classes/java/time/format/Parsed.java line 500: >> >>> 498: } >>> 499: } >>> 500: } >> >> Looking at the existing logic, the `AMPM_OF_DAY` field is completely ignored >> if there is no `HOUR_OF_AMPM` field. Thus, there is no validation to check >> `AMPM_OF_DAY` against `HOUR_OF_DAY`. This seems odd. (AMPM_OF_DAY = 0 and >> HOUR_OF_DAY=18 does not look like it throws an exception, when it probably >> should). >> >> On solution would be for `AMPM_OF_DAY` to be resolved to a day period at >> line 427, checking for conflicts with any parsed day period. (a small bug >> fix behavioural change) > > There are cases where a period crosses midnight, e.g., 23:00-04:00 so it > cannot validate am/pm, so I decided to just override ampm with dayperiod > without validating. Pulling on this a little more. As the PR stands, it seems that if the user passes in text with just a day-period of "AM" they get a `LocalTime` of 06:00 but if they pass in `AMPM_OF_DAY` of "AM" the get no `LocalTime` in the result. Is that right? If so, I don't think this is sustainable. Thats why I think `AMPM_OF_DAY` will have to be resolved to a dayPeriod of "am" or "pm". If dayPeriod is more precise than `AMPM_OF_DAY`, then dayPeriod can silently take precedence - PR: https://git.openjdk.java.net/jdk/pull/938
Re: RFR: 8255226: (tz) Upgrade time-zone data to tzdata2020d
On Mon, 2 Nov 2020 17:10:34 GMT, Naoto Sato wrote: >> Hi Guys, >> >> Please review the integration of tzdata2020d to JDK. >> >> Details regarding the change can be viewed at - >> https://mm.icann.org/pipermail/tz-announce/2020-October/62.html >> Bug: https://bugs.openjdk.java.net/browse/JDK-8255226 >> >> TestZoneInfo310.java test failure is addressed along with it. The last rule >> affects "Asia/Gaza" and "Asia/Hebron" and therefore excluded from the test. >> >> Regression Tests pass along with JCK. >> >> Please let me know if the changes are good to push. >> >> Thanks, >> Kiran > > test/jdk/sun/util/calendar/zi/TestZoneInfo310.java line 201: > >> 199: zid.equals("Iran") || // last rule mismatch >> 200: zid.equals("Asia/Gaza") || // last rule mismatch >> 201: zid.equals("Asia/Hebron")) { // last rule mismatch > > Can you explain why these zones are failing? The criteria for excluding > failing tests here is that the zone has negative dst and last rules beyond > 2037, and I don't think those newly excluded zones suffice those. It's probably these last rule what is causing the issue Rule Palestine 2020max - Mar Sat>=24 0:001:00S Rule Palestine 2020max - Oct Sat>=24 1:000 - The failure seen is ** Asia/Gaza : Asia/Gaza - SimpleTimeZone (NG) stz=java.util.SimpleTimeZone[id=Asia/Gaza,offset=720,dstSavings=360,useDaylight=true,startYear=0,startMode=2,startMonth=2,startDay=-1,startDayOfWeek=7,startTime=0,startTimeMode=0,endMode=2,endMonth=9,endDay=-1,endDayOfWeek=7,endTime=360,endTimeMode=0] stz0=java.util.SimpleTimeZone[id=Asia/Gaza,offset=720,dstSavings=360,useDaylight=true,startYear=0,startMode=3,startMonth=2,startDay=24,startDayOfWeek=7,startTime=0,startTimeMode=0,endMode=3,endMonth=9,endDay=24,endDayOfWeek=7,endTime=360,endTimeMode=0] - PR: https://git.openjdk.java.net/jdk/pull/1012
Re: RFR: 8255226: (tz) Upgrade time-zone data to tzdata2020d
On Mon, 2 Nov 2020 18:06:28 GMT, Kiran Sidhartha Ravikumar wrote: >> test/jdk/sun/util/calendar/zi/TestZoneInfo310.java line 201: >> >>> 199: zid.equals("Iran") || // last rule mismatch >>> 200: zid.equals("Asia/Gaza") || // last rule mismatch >>> 201: zid.equals("Asia/Hebron")) { // last rule mismatch >> >> Can you explain why these zones are failing? The criteria for excluding >> failing tests here is that the zone has negative dst and last rules beyond >> 2037, and I don't think those newly excluded zones suffice those. > > It's probably these last rule what is causing the issue > > Rule Palestine2020max - Mar Sat>=24 0:001:00 > S > Rule Palestine2020max - Oct Sat>=24 1:000 > - > > The failure seen is > > ** > Asia/Gaza : Asia/Gaza > - > SimpleTimeZone (NG) > > stz=java.util.SimpleTimeZone[id=Asia/Gaza,offset=720,dstSavings=360,useDaylight=true,startYear=0,startMode=2,startMonth=2,startDay=-1,startDayOfWeek=7,startTime=0,startTimeMode=0,endMode=2,endMonth=9,endDay=-1,endDayOfWeek=7,endTime=360,endTimeMode=0] > > stz0=java.util.SimpleTimeZone[id=Asia/Gaza,offset=720,dstSavings=360,useDaylight=true,startYear=0,startMode=3,startMonth=2,startDay=24,startDayOfWeek=7,startTime=0,startTimeMode=0,endMode=3,endMonth=9,endDay=24,endDayOfWeek=7,endTime=360,endTimeMode=0] My question is why it is failing. Have you figured it? The existing exceptions are either negative DST or last rule beyond 2037, which javazic cannot handle. The changes introduced with 2020d does not meet either of them. Unless we know why it is failing, we cannot be sure we can exclude it. - PR: https://git.openjdk.java.net/jdk/pull/1012
Integrated: 8211958: Broken links in java.desktop files
On Mon, 2 Nov 2020 09:04:39 GMT, Sergey Bylokhov wrote: > Most of the broken links were fixed already by the JDK-8225368 and JDK-8214817 > This change fix just a few. > > Also two cleanups are applied: > - The `../../java/awt/doc-files/` in some cases simplified to `doc-files/` > - The html links to the java classes via `` replaced by the `{@link > }` This pull request has now been integrated. Changeset: acb5f654 Author:Sergey Bylokhov URL: https://git.openjdk.java.net/jdk/commit/acb5f654 Stats: 43 lines in 9 files changed: 0 ins; 4 del; 39 mod 8211958: Broken links in java.desktop files Reviewed-by: aivanov - PR: https://git.openjdk.java.net/jdk/pull/998
Re: RFR: 8247781: Day periods support [v3]
On Mon, 2 Nov 2020 17:27:35 GMT, Stephen Colebourne wrote: >> Naoto Sato has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fixed exception messages. > > test/jdk/java/time/test/java/time/format/TestDateTimeFormatterBuilder.java > line 656: > >> 654: {"h B", "11 at night", 23}, >> 655: {"h B", "3 at night", 3}, >> 656: {"h B", "11 in the morning", 11}, > > Need tests for "51 in the morning" (which should parse in LENIENT as "3 in > the morning" plus 2 days, see how HOUR_OF_DAY=51 works in general. > > Similar issue with HOUR_OF_AMPM=3 and AMPM_OF_DAY=4. Fixed. - PR: https://git.openjdk.java.net/jdk/pull/938
Re: RFR: 8247781: Day periods support [v4]
> Hi, > > Please review the changes for the subject issue. This is to enhance the > java.time package to support day periods, such as "in the morning", defined > in CLDR. It will add a new pattern character 'B' and its supporting builder > method. The motivation and its spec are in this CSR: > > https://bugs.openjdk.java.net/browse/JDK-8254629 > > Naoto Naoto Sato has updated the pull request incrementally with three additional commits since the last revision: - Addressing https://github.com/openjdk/jdk/pull/938#discussion_r516147298 - Addressing https://github.com/openjdk/jdk/pull/938#discussion_r516123190 - Addressing https://github.com/openjdk/jdk/pull/938#discussion_r516138455 - Changes: - all: https://git.openjdk.java.net/jdk/pull/938/files - new: https://git.openjdk.java.net/jdk/pull/938/files/29c47afc..297acc94 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=938&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=938&range=02-03 Stats: 66 lines in 2 files changed: 53 ins; 3 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/938.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/938/head:pull/938 PR: https://git.openjdk.java.net/jdk/pull/938
Re: RFR: 8247781: Day periods support [v4]
On Mon, 2 Nov 2020 17:05:40 GMT, Stephen Colebourne wrote: >> MINUTE_OF_HOUR without HOUR_OF_DAY/HOUR_OF_AMPM may not make any sense, so >> it is only validated in updateCheckDayPeriodConflict. > > But can you get a CLDR rule that says "at night" before 05:30 and "In the > morning" from 05:30 onwards? If you can then I don't think it is handled, > because HOUR_OF_DAY and MINUTE_OF_HOUR are not used together when checking > against DayPeriod. CLDR allows rules that involve MINUTE_OF_HOUR. The validation I meant was within the `updateCheckDayPeriodConflict`: long hod = fieldValues.get(HOUR_OF_DAY); long moh = fieldValues.containsKey(MINUTE_OF_HOUR) ? fieldValues.get(MINUTE_OF_HOUR) : 0; long mod = hod * 60 + moh; if (!dayPeriod.includes(mod)) { throw new DateTimeException("Conflict found: Resolved time %02d:%02d".formatted(hod, moh) + " conflicts with " + dayPeriod); } which checks both hod/moh. moh is assumed zero if `MINUTE_OF_HOUR` does not exist. Are you suggesting `HOUR_OF_DAY` should also be assumed zero if it does not exist? >> There are cases where a period crosses midnight, e.g., 23:00-04:00 so it >> cannot validate am/pm, so I decided to just override ampm with dayperiod >> without validating. > > Pulling on this a little more. > > As the PR stands, it seems that if the user passes in text with just a > day-period of "AM" they get a `LocalTime` of 06:00 but if they pass in > `AMPM_OF_DAY` of "AM" the get no `LocalTime` in the result. Is that right? If > so, I don't think this is sustainable. > > Thats why I think `AMPM_OF_DAY` will have to be resolved to a dayPeriod of > "am" or "pm". If dayPeriod is more precise than `AMPM_OF_DAY`, then dayPeriod > can silently take precedence I implemented what you suggested here in the latest PR, but that would be a behavioral change which requires a CSR, as "AM" would be resolved to 06:00 which was not before. Do you think it would be acceptable? If so, I will reopen the CSR and describe the change. (In fact some TCK failed with this impl) - PR: https://git.openjdk.java.net/jdk/pull/938
Re: RFR: 8247781: Day periods support [v4]
On Mon, 2 Nov 2020 17:08:10 GMT, Stephen Colebourne wrote: >> src/java.base/share/classes/java/time/format/Parsed.java line 497: >> >>> 495: AMPM_OF_DAY.checkValidValue(ap); >>> 496: } >>> 497: updateCheckDayPeriodConflict(AMPM_OF_DAY, midpoint >>> / 720); >> >> No need to put `AMPM_OF_DAY` back in here because you've already resolved it >> to `HOUR_OF_DAY` and `MINUTE_OF_HOUR`. There probably does need to be >> validation to check that the day period agrees with the AM/PM value. > > Line can still be removed AFAICT Fixed. - PR: https://git.openjdk.java.net/jdk/pull/938
Re: RFR: 8255226: (tz) Upgrade time-zone data to tzdata2020d
On Mon, 2 Nov 2020 18:14:47 GMT, Naoto Sato wrote: >> It's probably these last rule what is causing the issue >> >> Rule Palestine 2020max - Mar Sat>=24 0:001:00 >> S >> Rule Palestine 2020max - Oct Sat>=24 1:000 >> - >> >> The failure seen is >> >> ** >> Asia/Gaza : Asia/Gaza >> - >> SimpleTimeZone (NG) >> >> stz=java.util.SimpleTimeZone[id=Asia/Gaza,offset=720,dstSavings=360,useDaylight=true,startYear=0,startMode=2,startMonth=2,startDay=-1,startDayOfWeek=7,startTime=0,startTimeMode=0,endMode=2,endMonth=9,endDay=-1,endDayOfWeek=7,endTime=360,endTimeMode=0] >> >> stz0=java.util.SimpleTimeZone[id=Asia/Gaza,offset=720,dstSavings=360,useDaylight=true,startYear=0,startMode=3,startMonth=2,startDay=24,startDayOfWeek=7,startTime=0,startTimeMode=0,endMode=3,endMonth=9,endDay=24,endDayOfWeek=7,endTime=360,endTimeMode=0] > > My question is why it is failing. Have you figured it? The existing > exceptions are either negative DST or last rule beyond 2037, which javazic > cannot handle. The changes introduced with 2020d does not meet either of > them. Unless we know why it is failing, we cannot be sure we can exclude it. Thanks for getting back Naoto, I believe this is a long-standing issue - https://bugs.openjdk.java.net/browse/JDK-8227293. Looking back at the integration of tzdata2019a/tzdata2019b, the same issue was addressed by updating the source code - https://hg.openjdk.java.net/jdk/jdk/rev/79036e5e744b#l13.1. Here is some history to the issue - https://mail.openjdk.java.net/pipermail/i18n-dev/2019-July/002887.html Please let me know your thoughts - PR: https://git.openjdk.java.net/jdk/pull/1012
Re: RFR: 8255226: (tz) Upgrade time-zone data to tzdata2020d
On Tue, 3 Nov 2020 00:00:26 GMT, Kiran Sidhartha Ravikumar wrote: >> My question is why it is failing. Have you figured it? The existing >> exceptions are either negative DST or last rule beyond 2037, which javazic >> cannot handle. The changes introduced with 2020d does not meet either of >> them. Unless we know why it is failing, we cannot be sure we can exclude it. > > Thanks for getting back Naoto, I believe this is a long-standing issue - > https://bugs.openjdk.java.net/browse/JDK-8227293. > > Looking back at the integration of tzdata2019a/tzdata2019b, the same issue > was addressed by updating the source code - > https://hg.openjdk.java.net/jdk/jdk/rev/79036e5e744b#l13.1. > > Here is some history to the issue - > https://mail.openjdk.java.net/pipermail/i18n-dev/2019-July/002887.html > > Please let me know your thoughts Should we then remove the hack in the ZoneInfoFile.java that excludes Gaza/Hebron instead? - PR: https://git.openjdk.java.net/jdk/pull/1012