> I will upload the patch once the bug is moved on to NEW state.
We don't make much distinction between UNCONFIRMED and NEW, so it's perfectly fine to work on an UNCONFIRMED bug.
Thanks for the info. Will upload the patch, once layout test is ready. Created attachment 127574 [details]
Patch to fix the clipping issue.
Patch to fix the clipping issue.
Comment on attachment 127574 [details] Patch to fix the clipping issue. Attachment 127574 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11539496 New failing tests: fast/borders/fieldsetBorderRadius.html fast/block/basic/fieldset-stretch-to-legend.html Created attachment 127691 [details]
Attachment showing actual output
Hi,
I executed these failing tests(pixel tests) on win-7 and the first test is passing, but the second test(fast/block/basic/fieldset-stretch-to-legend.html) is failing with 0.61% of difference in images. How ever Following are my observations on both the test results,
1. The difference is "actual image"(Attached actual image rendered through win launcher) has full border, where as the expected image has little clipped regions when ever the "legend" element touches left or right borders of fieldset.
2. I checked this behavior on all major browsers and only Chrome&Safari browser seems to be clipping the left and right borders.
3. Also, there is no specification saying that left and right borders should be clipped like the way Chrome&Safari browsers are doing right now.
So, i feel the present patch is also fixing these two tests and they need to be re-baselined, based on above data.
Comment on attachment 127691 [details]
Attachment showing actual output
Incorrect format
Created attachment 127692 [details]
Correct attachment showing un-clipped borders for fieldset-stretch-to-legend.html Test case
Created attachment 127791 [details]
New patch that updates previously failing pixel tests
Updated -expected files after re-baselining
fast/borders/fieldsetBorderRadius.html
fast/block/basic/fieldset-stretch-to-legend.html
Comment on attachment 127791 [details] New patch that updates previously failing pixel tests Attachment 127791 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11550150 New failing tests: fast/borders/fieldsetBorderRadius.html fast/block/basic/fieldset-stretch-to-legend.html Comment on attachment 127791 [details] New patch that updates previously failing pixel tests View in context: https://bugs.webkit.org/attachment.cgi?id=127791&action=review Thanks for taking this! The code basically looks OK. What's left is bureaucratic part... 1. It looks you don't set the mime type of the png files correctly. Please check your svn configuration. http://trac.webkit.org/wiki/UsingGitWithWebKit 2. On chromium bot (cr-linux) redness, you can mark "fail" to these failing test to make the bot OK. Someone from Chromium land will care about that then. http://trac.webkit.org/wiki/TestExpectations > Source/WebCore/rendering/RenderFieldset.cpp:165 > + LayoutUnit clipWidth = (max(static_cast<LayoutUnit>(style()->borderLeftWidth()), legend->width())); Apparently this change isn't required. Created attachment 127933 [details]
Patch to fix the clipping issue and updated chromiums test expectation file.
Thanks a lot for the comments. I have fixed all the issues mentioned by you, and updated test_expectations.txt file of chromium platform.
Created attachment 128107 [details]
Updated patch as there was merge conflict in test_expectations.txt file
Comment on attachment 128107 [details] Updated patch as there was merge conflict in test_expectations.txt file Attachment 128107 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11520393 Hi Morita, I have done the changes as mentioned by you, but it still looks like i am missing something. Can you please suggest me why i am seeing following line(in output) "Regressions: Unexpected no expected results found : (1) fast/forms/fieldset-legend-padding-unclipped-fieldset-border.html = MISSING" though i have attached the expected results file. (In reply to comment #14) > Hi Morita, > > I have done the changes as mentioned by you, but it still looks like i am missing something. Can you please suggest me why i am seeing following line(in output) > > "Regressions: Unexpected no expected results found : (1) > fast/forms/fieldset-legend-padding-unclipped-fieldset-border.html = MISSING" > > though i have attached the expected results file. The error message in this case might be a bit under-informative; your test is not a text-only test (i.e., it will generate a PNG file), but there is no -expected.png file in your patch as far as I can see. I suspect the bot is complaining about that. Created attachment 128403 [details]
Patch with -expected png file.
Thanks for the info, I have generated the -expected png file for the test and uploading the new patch.
Created attachment 128469 [details]
Patch after resolving merge conflict on chromium/test_expectations.txt
Created attachment 128476 [details]
Patch after resolving merge conflict on chromium/test_expectations.txt and removing style issue
Comment on attachment 128476 [details] Patch after resolving merge conflict on chromium/test_expectations.txt and removing style issue Attachment 128476 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11610266 New failing tests: fast/forms/fieldset-legend-padding-unclipped-fieldset-border.html Hi, I do not see in the attached output that the test is failing, but in the last comments i see that test is resulting in regression, can some please tell me whats going on, i have spent more time with patch than on bug fix. I just want to know where i am going terribly wrong that its almost 4 days and cr-linux bot is still RED. Thanks in anticipation. Attachment 127692 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files']" exit_code: 1
Total errors found: 0 in 0 files
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files']" exit_code: 1
Total errors found: 0 in 0 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Hi, The error report does'nt really show on what file did it apply check-webkit-style, so what kind of bug should i file with this information?. (In reply to comment #22) > Hi, > > The error report does'nt really show on what file did it apply check-webkit-style, so what kind of bug should i file with this information?. Hi, I would file an error along the lines of "style bot failed but check-webkit-style didn't report any errors" :). Also, I have no explanation for the "new failing test" result you got in comment #19, since the link to http://queues.webkit.org/results/11610266 doesn't even mention that file. Adam, Eric, either of you have any idea what's going on with the bots here? SravanKumar, it might be worth trying again ... unfortunately, there's no "retry" button on the bugzilla form, so you would need to clear the review? flag from your attachment and re-upload it. Sorry! I realize this whole process is a bit frustrating; there is definitely still a learning curve to getting all of the contributor tools working for you, even when you don't hit bugs. Sorry for the noise. I'm working on replacing some guts in the style-queue and was running some experiments today. :), some response in between is tempting enough to keep persisting with trials. Ok, any how before i upload the patch again, i need one quick clarification, one of the patch has following lines in it, will it go through style bot? Index: LayoutTests/fast/forms/fieldset-legend-padding-unclipped-fieldset-border-expected.png =================================================================== Cannot display: file marked as a binary type. svn:mime-type = image/png Index: LayoutTests/fast/forms/fieldset-legend-padding-unclipped-fieldset-border-expected.png =================================================================== - LayoutTests/fast/forms/fieldset-legend-padding-unclipped-fieldset-border-expected.png (revision 0) +++ LayoutTests/fast/forms/fieldset-legend-padding-unclipped-fieldset-border-expected.png (working copy) Property changes on: LayoutTests/fast/forms/fieldset-legend-padding-unclipped-fieldset-border-expected.png ___________________________________________________________________ Added: svn:mime-type ## -0,0 +1 ## +image/png I am sure it wont go through, so i 1. back up -expected.png 2. svn delete "xyz-expected.png" 3. touch "xyz-expected.png" 4. svn add "xyz-expected.png" 5. re-baseline to regenerate "xyz-expected.png" 6. Now re-generate the patch again, Then i dont see above lines in the patch, but patch size is increased and png file has some non-readable characters in patch file. And i upload this file as patch. So based on my above explanation can you let me know how to counter these issues and create a safe patch and upload it. I am using cygwin(CYGWIN_NT-6.1-WOW64) svn1.7 on Win7 Apart from that last query, Adam, please let me know when is it safe to upload the patch? > please let me know when is it safe to upload the patch?
Please feel free to upload patches whenever you want.
Created attachment 128985 [details]
Patch with svn mime type set
Created attachment 129027 [details]
Patch
Hi,
i found that when we run 'svn-create-patch', this script executes svn diff on the png file in one of the iterations. When i tried to do the same exclusively from cygwin command line and normal command prompt,
i.e,
svn diff --diff-cmd diff LayoutTests/platform/win/fast/forms/fieldset-legend-
padding-unclipped-fieldset-border-expected.png
i see that following as the footer
___________________________________________________________________
Added: svn:mime-type
## -0,0 +1 ##
+image/png
And on chasing .Tools/Scripts/svn-apply i found that script is actually dying because it does'nt get propertyValue(from VCSUtils.pm file), as the condition in while loop is not getting satisisfied (while (defined($_) && /$svnPropertyValueStartRegEx/) {). It is expecting any one of /^ (\+|-|Merged|Reverse-merged) ([^\r\n]+)/ But $_ is ## -0,0 +1 ##.
But since this is a new file it will never be Merged or Reverse-merged, and i dont see any + or - with mime-type:image/png .
Hence my patch will never go through style bots, and i am sorry for uploading so many failing patches.
I just want to know if my cygwin settings are wrong? or svn configuration settings are wrong? or do i have to leave this and find an alternate method to upload the patch?.
Please kindly let me know what else can be done to upload the patch.
Created attachment 129674 [details]
Created patch on MAC
Footer issue found previously is not visible when patch is generated through MAC Machine(LION.)
Hi, Finally i am able to see the bots "Green" :-), can some one please kindly review it? Comment on attachment 129674 [details] Created patch on MAC Rejecting attachment 129674 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: pped-fieldset-border.html patching file LayoutTests/platform/chromium/test_expectations.txt Hunk #1 FAILED at 4437. 1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/chromium/test_expectations.txt.rej patching file LayoutTests/platform/win/fast/forms/fieldset-legend-padding-unclipped-fieldset-border-expected.txt Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Dirk Pranke']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ Full output: http://queues.webkit.org/results/11800073 Committed r109612: <http://trac.webkit.org/changeset/109612> Comment on attachment 129674 [details] Created patch on MAC landed by hand as http://trac.webkit.org/changeset/109612 . Thanks a lot for landing the patch, and i have filed a bug on svn footer issue, with decent information at https://bugs.webkit.org/show_bug.cgi?id=80104. |
Created attachment 127137 [details] Test case to validate the buggy behaviour described above Overview: On applying padding style information to fieldset's "legend" element, the bottom border(of fieldset) gets clipped-off in Safari and Chrome. There is no such issue on FF & IE irrespective of padding value. Steps to Reproduce: 1. PFA test case to reproduce the same(fieldset-legend-padding.html) 2. Run it either on Safari or Chrome. On analyzing further, I found that clipping of legend element is buggy in WebKit, and i may have the fix for it. I will upload the patch once the bug is moved on to NEW state.