Add feature flag that enables lang attribute for form controls in LayoutTests
Created attachment 163845 [details] Patch
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment on attachment 163845 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=163845&action=review > Source/WebCore/ChangeLog:13 > + (WebCore::RuntimeEnabledFeatures::langAttributeForFormControlsEnabled): This name doesn't explain the feature well. Form controls can have lang attributes even now. It enabled lang-attribute-aware-form-control-UI, right? > Source/WebKit/chromium/public/WebRuntimeFeatures.h:165 > + WEBKIT_EXPORT static void enableLangAttributeForFormControls(bool); > + WEBKIT_EXPORT static bool isLangAttributeForFormControlsEnabled(); I prefer enabling the feature in InternalSettings to in public WebKit API because this feature is for testing.
Created attachment 163858 [details] Patch
Comment on attachment 163858 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=163858&action=review > Source/WebKit/chromium/ChangeLog:13 > + * public/WebRuntimeFeatures.h: > + (WebRuntimeFeatures): > + * src/WebRuntimeFeatures.cpp: > + (WebKit::WebRuntimeFeatures::enableLangAttributeForFormControls): > + (WebKit): > + (WebKit::WebRuntimeFeatures::isLangAttributeAwareFormControlUIEnabled): These changes are not needed any more. > Source/WebCore/testing/InternalSettings.cpp:649 > + UNUSED_PARAM(ec); So, you don't need to specify rases(DOMException) in the IDL. > Source/WebCore/testing/InternalSettings.cpp:651 > +} You need to update InternalSettings::Backup::Backup() and restoreTo().
Comment on attachment 163858 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=163858&action=review > Source/WebCore/ChangeLog:3 > + Add feature flag that enables lang attribute aware form controls UI in LayoutTests Do we call these "feature flags"? The title of the bug made me think of an ENABLE macro.
(In reply to comment #6) > Do we call these "feature flags"? The title of the bug made me think of an ENABLE macro. I'm sorry. Past change logs seem to call it the runtime flag.
Created attachment 163971 [details] Patch
Comment on attachment 163971 [details] Patch Attachment 163971 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13841503
Comment on attachment 163971 [details] Patch Attachment 163971 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13841527
Created attachment 164015 [details] Patch
Comment on attachment 164015 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164015&action=review > Source/WebCore/bindings/generic/RuntimeEnabledFeatures.h:264 > + static void setLangAttributeAwareFormControlUIEnabled(bool isEnabled) { isLangAttributeAwareFormControlUIEnabled = isEnabled; } Please add a warning comment that this should be enabled only for testing.
Comment on attachment 164015 [details] Patch Attachment 164015 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13852247
Created attachment 164040 [details] Patch
Created attachment 164041 [details] Patch
Comment on attachment 164041 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164041&action=review > Source/WebCore/platform/text/LocaleWin.h:-97 > - Vector<String> m_shortMonthLabels; > - Vector<String> m_monthLabels; > #if ENABLE(CALENDAR_PICKER) > - Vector<String> m_weekDayShortLabels; This part looks unrelated to this bug.
Created attachment 164042 [details] Patch
Comment on attachment 164042 [details] Patch ok
Comment on attachment 164042 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164042&action=review > Source/WebCore/bindings/generic/RuntimeEnabledFeatures.h:264 > + static void setLangAttributeAwareFormControlUIEnabled(bool isEnabled) { isLangAttributeAwareFormControlUIEnabled = isEnabled; } Please add a warning here.
Created attachment 164068 [details] Patch
Comment on attachment 164068 [details] Patch Rejecting attachment 164068 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 ERROR: /mnt/git/webkit-commit-queue/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/13850476
Comment on attachment 164068 [details] Patch Clearing flags on attachment: 164068 Committed r128583: <http://trac.webkit.org/changeset/128583>
All reviewed patches have been landed. Closing bug.
Hello, Currently there are one of two revisions that are causing the Windows test bots to exit early after 20 crashes. This is one possible candidate. Can you see if this would have caused a large quantity of accessibility tests on Windows 7 to start failing all of a sudden? The Windows bots have suddenly gotten into a terrible state so if you could verify this quickly I'd be greatly appreciative. Thanks
(In reply to comment #24) > Hello, > Currently there are one of two revisions that are causing the Windows test bots to exit early after 20 crashes. This is one possible candidate. > > Can you see if this would have caused a large quantity of accessibility tests on Windows 7 to start failing all of a sudden? The Windows bots have suddenly gotten into a terrible state so if you could verify this quickly I'd be greatly appreciative. > > Thanks Roger, It seems the change in this bug was the trigger. http://build.webkit.org/results/Apple%20Win%20XP%20Debug%20(Tests)/r128619%20(44630)/accessibility/accessibility-node-memory-management-crash-log.txt I'm not sure why Windows crashes at such code. I'm investigating.
My test log indicates that the 20 crashes started sometime between 128583 and 128585 128585 #36257 Sep 14 04:23 Build successful #27689 Sep 14 04:41 Failed exiting early after 20 crashes, 0 web process crashes, and 0 timeouts. 20 tests run. 20 test cases (100%) crashed 1 unit tests failed or timed out webkitpy-test 128584 #36256 Sep 14 04:19 Build successful 128583 #36255 Sep 14 04:11 Build successful #27688 Sep 14 04:20 Failed layout-test 1 unit tests failed or timed out 3 python tests failed 128581 #36254 Sep 14 03:58 Build successful #27687 Sep 14 04:02 Failed 66 test cases (<1%) had incorrect layout 3 test cases (<1%) timed out 1 unit tests failed or timed out 3 python tests failed
We successfully avoided the Windows issue by http://trac.webkit.org/changeset/128629 though I don't understand the root cause yet.
Hmm, well thanks for fixing things! Bots are happy(er) again. Much appreciated.
Hi, Apple Windows port is broken again (although only when running DRT locally) and the workaround seems to have been removed...can I add it back in?
(In reply to comment #29) > Hi, Apple Windows port is broken again (although only when running DRT locally) and the workaround seems to have been removed...can I add it back in? Work around was removed in a later patch in favor of a patch that theoretically fixed things on Windows, but unfortunately not when run locally. Continuing my investigation in appropriate bug.
(In reply to comment #30) > (In reply to comment #29) > > Hi, Apple Windows port is broken again (although only when running DRT locally) and the workaround seems to have been removed...can I add it back in? > > Work around was removed in a later patch in favor of a patch that theoretically fixed things on Windows, but unfortunately not when run locally. Continuing my investigation in appropriate bug. Have you never tried local DRT after http://trac.webkit.org/changeset/128850 ? Anyway, please file another bug.
Apparently not. I was working on stuff that didn't require me to use it for a while. It's also running fine on the test bots, so I didn't think anything was going wrong until just recently.