Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
144513002:
[GFX] Add hostname elider. Add ability to elide strings at the beginning. (Closed)
|
Created:
6 years, 11 months ago by Greg Billock Modified:
6 years, 10 months ago CC:
chromium-reviews, jamesr Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
Description[GFX] Add hostname elider. Add ability to elide strings at the beginning.
[email protected]
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=248726
Patch Set 1 #Patch Set 2 : . #
Total comments: 30
Patch Set 3 : comments, test #
Total comments: 20
Patch Set 4 : tweaks #Patch Set 5 : tweaks #Patch Set 6 : Account for ios emulator #Patch Set 7 : Follow ElideURL to chrome/browser/ui #Patch Set 8 : update namespace, add test for short target lengths #
Total comments: 2
Patch Set 9 : Add comment #Patch Set 10 : Mac compile workaround #Patch Set 11 : Disable UI test on Android #
Messages
Total messages: 39 (0 generated)
Sorry that I have not been working on Chromium codebase for a long time. Could you please check who is the owner and/or the most recent modifier on those files and ask them for a view?
On 2014/01/24 21:19:04, xji wrote: > Sorry that I have not been working on Chromium codebase for a long time. Could > you please check who is the owner and/or the most recent modifier on those files > and ask them for a view? Thanks, will do.
On 2014/01/24 21:54:22, Greg Billock wrote: > On 2014/01/24 21:19:04, xji wrote: > > Sorry that I have not been working on Chromium codebase for a long time. Could > > you please check who is the owner and/or the most recent modifier on those > files > > and ask them for a view? > > Thanks, will do. Rerouting to msw
https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider.cc File ui/gfx/text_elider.cc (right): https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider.cc#ne... ui/gfx/text_elider.cc:401: // Get domain and registry information from the URL. Rather than copy ElideURL code, can you split that up into a reusable helper? https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider.cc#ne... ui/gfx/text_elider.cc:431: float subdomain_width = available_pixel_width - pixel_width_url_domain; Couldn't this potentially be negative? https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider.h File ui/gfx/text_elider.h (right): https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider.h#new... ui/gfx/text_elider.h:101: // the given width. The function will never elide past the TLD+1 point, nit: giving an example here would help clarify your meaning. https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... File ui/gfx/text_elider_unittest.cc (right): https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:66: void RunUrlTestHost(Testcase* testcases, size_t num_testcases) { This is only used in one test, you may as well inline it there. https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:70: if (url.is_valid()) nit: why not just use the possibly_invalid_spec() for logging? https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:71: LOG(INFO) << "url " << url.spec(); Tests shouldn't be this verbose; use VLOG or SCOPED_TRACE or similar. https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:72: // Should we test with non-empty language list? Make a decision here, or make this a TODO. https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:272: TEST(TextEliderTest, TestHostEliding) { How about some URLs with paths, usernames, passwords, ports, queries, escaped characters, https/ftp/file/data/js schemes, etc.? https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:422: const char* kEllipsis = "\xE2\x80\xA6"; Why not use const std::string kEllipsisStr(kEllipsis); like above? https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:436: { "Test123", kTestWidth, "\xE2\x80\xA6" "23" }, I suspect this test case is too fragile to changes in the fonts or per-platform differences, you should explicitly measure the length of "...23" to make this case more robust. Also, can you concatenate "23" to kEllipsis here? https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:509: base::string16 long_string_beginning( I think it would make a lot more sense to either: 1) Make a separate ELIDE_AT_BEGINNING test cases array (like ELIDE_AT_BEGINNING). 2) Make UTF16Testcase have the output for all three elide modes. https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:538: ElideText( optional nit: you can wrap args here. https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:575: ELIDE_AT_END).size()); YIKES! Shouldn't this and line 578 be using ELIDE_IN_MIDDLE? That seems like a pretty bad oversight... and a good reason to compare the actual output strings, not their length...
https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider.cc File ui/gfx/text_elider.cc (right): https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider.cc#ne... ui/gfx/text_elider.cc:401: // Get domain and registry information from the URL. On 2014/01/25 02:38:24, msw wrote: > Rather than copy ElideURL code, can you split that up into a reusable helper? Done. https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider.cc#ne... ui/gfx/text_elider.cc:431: float subdomain_width = available_pixel_width - pixel_width_url_domain; On 2014/01/25 02:38:24, msw wrote: > Couldn't this potentially be negative? Yes. I'm depending on ElideText to just return an ellipsis in that case. Is that fragile? https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider.h File ui/gfx/text_elider.h (right): https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider.h#new... ui/gfx/text_elider.h:101: // the given width. The function will never elide past the TLD+1 point, On 2014/01/25 02:38:24, msw wrote: > nit: giving an example here would help clarify your meaning. Done. https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... File ui/gfx/text_elider_unittest.cc (right): https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:66: void RunUrlTestHost(Testcase* testcases, size_t num_testcases) { On 2014/01/25 02:38:24, msw wrote: > This is only used in one test, you may as well inline it there. OK, done. https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:70: if (url.is_valid()) On 2014/01/25 02:38:24, msw wrote: > nit: why not just use the possibly_invalid_spec() for logging? makes sense. I planned on just deleting that (now done). https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:71: LOG(INFO) << "url " << url.spec(); On 2014/01/25 02:38:24, msw wrote: > Tests shouldn't be this verbose; use VLOG or SCOPED_TRACE or similar. removed https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:72: // Should we test with non-empty language list? On 2014/01/25 02:38:24, msw wrote: > Make a decision here, or make this a TODO. I think for domains it doesn't matter. Removed. https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:272: TEST(TextEliderTest, TestHostEliding) { On 2014/01/25 02:38:24, msw wrote: > How about some URLs with paths, usernames, passwords, ports, queries, escaped > characters, https/ftp/file/data/js schemes, etc.? I'll add one. The code specifically just ignores them, so I don't think we need a lot of them. https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:422: const char* kEllipsis = "\xE2\x80\xA6"; On 2014/01/25 02:38:24, msw wrote: > Why not use const std::string kEllipsisStr(kEllipsis); like above? Done. https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:436: { "Test123", kTestWidth, "\xE2\x80\xA6" "23" }, On 2014/01/25 02:38:24, msw wrote: > I suspect this test case is too fragile to changes in the fonts or per-platform > differences, you should explicitly measure the length of "...23" to make this > case more robust. Also, can you concatenate "23" to kEllipsis here? Done. https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:509: base::string16 long_string_beginning( On 2014/01/25 02:38:24, msw wrote: > I think it would make a lot more sense to either: > 1) Make a separate ELIDE_AT_BEGINNING test cases array (like > ELIDE_AT_BEGINNING). > 2) Make UTF16Testcase have the output for all three elide modes. Made a separate test. https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:538: ElideText( On 2014/01/25 02:38:24, msw wrote: > optional nit: you can wrap args here. moved to separate test https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:575: ELIDE_AT_END).size()); On 2014/01/25 02:38:24, msw wrote: > YIKES! Shouldn't this and line 578 be using ELIDE_IN_MIDDLE? That seems like a > pretty bad oversight... and a good reason to compare the actual output strings, > not their length... I think you're right. I'm skeptical about this test to begin with. A completely uniform string seems like too easy a case to mess up. I do think comparing strings is better -- I did that for ElideTextEllipsisFront.
https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider.cc File ui/gfx/text_elider.cc (right): https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider.cc#ne... ui/gfx/text_elider.cc:431: float subdomain_width = available_pixel_width - pixel_width_url_domain; On 2014/01/27 19:11:30, Greg Billock wrote: > On 2014/01/25 02:38:24, msw wrote: > > Couldn't this potentially be negative? > > Yes. I'm depending on ElideText to just return an ellipsis in that case. Is that > fragile? It's not unreasonable, but not defined function behavior. It'll apparently return an empty string. I suppose this is fine, but the moderately expensive string measurements could possibly be avoided for efficiency. https://code.google.com/p/chromium/codesearch#chromium/src/ui/gfx/text_elider... https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... File ui/gfx/text_elider_unittest.cc (right): https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:575: ELIDE_AT_END).size()); On 2014/01/27 19:11:30, Greg Billock wrote: > On 2014/01/25 02:38:24, msw wrote: > > YIKES! Shouldn't this and line 578 be using ELIDE_IN_MIDDLE? That seems like a > > pretty bad oversight... and a good reason to compare the actual output > strings, > > not their length... > > I think you're right. I'm skeptical about this test to begin with. A completely > uniform string seems like too easy a case to mess up. > > I do think comparing strings is better -- I did that for ElideTextEllipsisFront. Please file a bug for the remaining length comparisons and add TODOs; You can assign the bug to me if you wish. https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider.cc File ui/gfx/text_elider.cc (right): https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider.cc#n... ui/gfx/text_elider.cc:117: const base::string16 kWwwPrefix = UTF8ToUTF16("www."); nit: make this static? https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... File ui/gfx/text_elider_unittest.cc (right): https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... ui/gfx/text_elider_unittest.cc:19: #include "url/url_canon.h" nit q: why is this added? https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... ui/gfx/text_elider_unittest.cc:259: {"http://google.com", "google.com"}, optional nit: ElideHost(GURL(testcases[i].input), FontList(), available_width)); https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... ui/gfx/text_elider_unittest.cc:261: {"http://reallyreallyreallylongdomainname.com", "reallyreallyreallylongdomainname.com"}, nit: limit these lines to 80 chars; 261, 262 https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... ui/gfx/text_elider_unittest.cc:272: const GURL url(testcases[i].input); nit: optionally inline below. https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... ui/gfx/text_elider_unittest.cc:414: //const char* kEllipsis = "\xE2\x80\xA6"; Remove commented code. https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... ui/gfx/text_elider_unittest.cc:424: { "", 0, base::string16() }, nit: optionally space these out to align between lines. https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... ui/gfx/text_elider_unittest.cc:435: cases[i].width, ELIDE_AT_BEGINNING); nit: indent to align after the open paren. https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... ui/gfx/text_elider_unittest.cc:504: {data_scheme + ten_a, data_scheme + ten_a}, nit: Add spaces inside the curly braces; ditto elsewhere. https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... ui/gfx/text_elider_unittest.cc:569: font_list, nit: optionally wrap onto the line above.
https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider.cc File ui/gfx/text_elider.cc (right): https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider.cc#ne... ui/gfx/text_elider.cc:431: float subdomain_width = available_pixel_width - pixel_width_url_domain; On 2014/01/28 19:59:51, msw wrote: > On 2014/01/27 19:11:30, Greg Billock wrote: > > On 2014/01/25 02:38:24, msw wrote: > > > Couldn't this potentially be negative? > > > > Yes. I'm depending on ElideText to just return an ellipsis in that case. Is > that > > fragile? > > It's not unreasonable, but not defined function behavior. It'll apparently > return an empty string. I suppose this is fine, but the moderately expensive > string measurements could possibly be avoided for efficiency. > https://code.google.com/p/chromium/codesearch#chromium/src/ui/gfx/text_elider... ok, sounds like a plan. https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... File ui/gfx/text_elider_unittest.cc (right): https://codereview.chromium.org/144513002/diff/60001/ui/gfx/text_elider_unitt... ui/gfx/text_elider_unittest.cc:575: ELIDE_AT_END).size()); On 2014/01/28 19:59:51, msw wrote: > On 2014/01/27 19:11:30, Greg Billock wrote: > > On 2014/01/25 02:38:24, msw wrote: > > > YIKES! Shouldn't this and line 578 be using ELIDE_IN_MIDDLE? That seems like > a > > > pretty bad oversight... and a good reason to compare the actual output > > strings, > > > not their length... > > > > I think you're right. I'm skeptical about this test to begin with. A > completely > > uniform string seems like too easy a case to mess up. > > > > I do think comparing strings is better -- I did that for > ElideTextEllipsisFront. > > Please file a bug for the remaining length comparisons and add TODOs; You can > assign the bug to me if you wish. 338836 https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider.cc File ui/gfx/text_elider.cc (right): https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider.cc#n... ui/gfx/text_elider.cc:117: const base::string16 kWwwPrefix = UTF8ToUTF16("www."); On 2014/01/28 19:59:51, msw wrote: > nit: make this static? Done. https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... File ui/gfx/text_elider_unittest.cc (right): https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... ui/gfx/text_elider_unittest.cc:19: #include "url/url_canon.h" On 2014/01/28 19:59:51, msw wrote: > nit q: why is this added? Removed. I think in an early take I was trying to pass strings instead of GURL, and that caused trouble. https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... ui/gfx/text_elider_unittest.cc:259: {"http://google.com", "google.com"}, On 2014/01/28 19:59:51, msw wrote: > optional nit: ElideHost(GURL(testcases[i].input), FontList(), available_width)); Done. https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... ui/gfx/text_elider_unittest.cc:261: {"http://reallyreallyreallylongdomainname.com", "reallyreallyreallylongdomainname.com"}, On 2014/01/28 19:59:51, msw wrote: > nit: limit these lines to 80 chars; 261, 262 Done. https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... ui/gfx/text_elider_unittest.cc:272: const GURL url(testcases[i].input); On 2014/01/28 19:59:51, msw wrote: > nit: optionally inline below. Done. https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... ui/gfx/text_elider_unittest.cc:414: //const char* kEllipsis = "\xE2\x80\xA6"; On 2014/01/28 19:59:51, msw wrote: > Remove commented code. Done. https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... ui/gfx/text_elider_unittest.cc:424: { "", 0, base::string16() }, On 2014/01/28 19:59:51, msw wrote: > nit: optionally space these out to align between lines. Done. https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... ui/gfx/text_elider_unittest.cc:435: cases[i].width, ELIDE_AT_BEGINNING); On 2014/01/28 19:59:51, msw wrote: > nit: indent to align after the open paren. Done. https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... ui/gfx/text_elider_unittest.cc:504: {data_scheme + ten_a, data_scheme + ten_a}, On 2014/01/28 19:59:51, msw wrote: > nit: Add spaces inside the curly braces; ditto elsewhere. Done. https://codereview.chromium.org/144513002/diff/130001/ui/gfx/text_elider_unit... ui/gfx/text_elider_unittest.cc:569: font_list, On 2014/01/28 19:59:51, msw wrote: > nit: optionally wrap onto the line above. Done.
LGTM
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/[email protected]/144513002/170001
Retried try job too often on mac_rel for step(s) remoting_unittests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/[email protected]/144513002/170001
Retried try job too often on ios_dbg_simulator for step(s) ui_unittests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/[email protected]/144513002/190001
On 2014/01/29 22:10:07, I haz the power (commit-bot) wrote: > CQ is trying da patch. Follow status at > https://chromium-status.appspot.com/cq/gbillock%40chromium.org/144513002/190001 Note: all the lengths check out for iOS -- the length limit isn't being violated, so the text width calculation has jitter in the test compared to the calculation in the production code, at least for some strings. This may just be for the emulator fonts used, or for reasons of inaccuracy in the text splicing.
Retried try job too often on linux_chromeos_clang for step(s) compile http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
On 2014/01/30 07:36:23, I haz the power (commit-bot) wrote: > Retried try job too often on linux_chromeos_clang for step(s) compile > http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro... +sky for chrome/browser/ui
On 2014/01/30 17:10:17, Greg Billock wrote: > On 2014/01/30 07:36:23, I haz the power (commit-bot) wrote: > > Retried try job too often on linux_chromeos_clang for step(s) compile > > > http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro... > > +sky for chrome/browser/ui OK, sorry, autocomplete error. Now adjusted. sky, jamesr, this last patch follows ElideURL to chrome/browser/ui for hostname eliding.
LGTM https://codereview.chromium.org/144513002/diff/230001/chrome/browser/ui/elide... File chrome/browser/ui/elide_url.cc (right): https://codereview.chromium.org/144513002/diff/230001/chrome/browser/ui/elide... chrome/browser/ui/elide_url.cc:72: void SplitHost(const GURL& url, Add description as to what this does.
https://codereview.chromium.org/144513002/diff/230001/chrome/browser/ui/elide... File chrome/browser/ui/elide_url.cc (right): https://codereview.chromium.org/144513002/diff/230001/chrome/browser/ui/elide... chrome/browser/ui/elide_url.cc:72: void SplitHost(const GURL& url, On 2014/02/03 17:25:28, sky wrote: > Add description as to what this does. Done.
The CQ bit was checked by [email protected]
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/[email protected]/144513002/270001
The CQ bit was unchecked by [email protected]
Retried try job too often on mac_rel for step(s) app_list_unittests, cacheinvalidation_unittests, cc_unittests, check_deps, check_deps2git, chromedriver_unittests, components_unittests, content_browsertests, content_unittests, crypto_unittests, google_apis_unittests, gpu_unittests, ipc_tests, jingle_unittests, media_unittests, message_center_unittests, nacl_integration, ppapi_unittests, printing_unittests, remoting_unittests, sql_unittests, sync_integration_tests, sync_unit_tests, telemetry_perf_unittests, telemetry_unittests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...
CQ bit was unchecked on CL. Ignoring.
CQ bit was unchecked on CL. Ignoring.
The CQ bit was checked by [email protected]
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/[email protected]/144513002/560001
CQ bit was unchecked on CL. Ignoring.
CQ bit was unchecked on CL. Ignoring.
The CQ bit was checked by [email protected]
Failed to trigger a try job on android_dbg HTTP Error 400: Bad Request
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/[email protected]/144513002/720001
The CQ bit was unchecked by [email protected]
Retried try job too often on linux_rel for step(s) browser_tests, sync_integration_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...
The CQ bit was checked by [email protected]
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/[email protected]/144513002/720001
Message was sent while issue was closed.
Change committed as 248726
Message was sent while issue was closed.
CQ bit was unchecked on CL. Ignoring. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
