Created attachment 61504 [details] Screenshot

Could not reproduce on Windows, unless the bug has been introduced really lately (my Qt HEAD dates of June-30). Marking as blocking the release since I believe it would be a nice to have to polish the Qt documentation experience. Comment or remove the block if you don't think it is worth fixing now.

Confirmed with Qt's X11 paint engine.

(In reply to comment #3) > Confirmed with Qt's X11 paint engine. (Does not happen with the raster engine.)

Created attachment 62711 [details] Patch which solving the problem with artefacts I have analyzed this problem two days ago. The cause of proble is - the line draw otside of bounding rect of graphics item (e.g. links in webkit). If we set line thihckness to 0.99 instead 1.00, we will solve this probleb. The second solution may be next: in the file webkit/WebCore/rendering/InlineTextBox.cpp at line 712 in code context->drawLineForText(IntPoint(tx, ty + baseline + 1), width, isPrinting); need to remove ' + 1', change this code to context->drawLineForText(IntPoint(tx, ty + baseline), width, isPrinting); But this solution may look poor with 'raster' or 'runtime' graphicsystems.

Sorry :-) 'proble' and 'probleb' must read as 'problem' in previous message.

(In reply to comment #5) > context->drawLineForText(IntPoint(tx, ty + baseline + 1), width, isPrinting); This looks correct to me. The font height is ascent + descent + 1. The +1 is justified here. It looks more like we are not reporting the correct size somewhere in QFontMetrics for X11.

> It looks more like we are not reporting the correct size somewhere in > QFontMetrics for X11. I agree. Therefore, I propose to change the line thickness from 1.00 to 0.99. It will be looks like in most cases. It will be looks like in most cases. See the patch.

(In reply to comment #8) > Therefore, I propose to change the line thickness from 1.00 to 0.99. This needs more explanations. What is the origin of the issue and why changing the thickness is the solution?

After I determined that the 'ascent + descent + 1' without ' + 1' eliminates the artifacts, I began to experiment with the values of 'margin' and 'padding' in the CSS. I found that the values of 'margin' greater than 0 (eg 1) also eliminates the artifacts. I think that bounding rect is higer in this case. Next, I tried to change the line thickness to bigger value - 2.0f - artifacts became thicker too. Then I set thickness to 0.99 and artifacts do not appear. I think that, perhaps this is due to the conversion of real coordinates in integer coordinates inside X-server.

So... It may be not clean and final solution of the problem, but it works :-) And this patch may tell you more right solution of the problem. I hope :-)

The bug was in Qt. The rendering of lines was different between the X11 graphic system and the other graphics systems. The patch ebbab30af417dfbf3df47dec15c0e2f8d6a30fa6 changes the behavior of Qt.

What is the patch ebbab30af417dfbf3df47dec15c0e2f8d6a30fa6? Where I can look it? Because I couldn't find commint 'ebbab30af417dfbf3df47dec15c0e2f8d6a30fa6 in Qt git repository. I try compile Qt from yesterday git repository (last commit 11468d1086315c2d4f77f3747488e423d499bf56) and the problem still exists. Moving the mouse fast over links still leave a trail with underlined links.

(In reply to comment #13) > What is the patch ebbab30af417dfbf3df47dec15c0e2f8d6a30fa6? Where I can look it? Here you go: http://qt.gitorious.org/+qt-developers/qt/staging/commit/ebbab30af417dfbf3df47dec15c0e2f8d6a30fa6 Patches are in a staging area before going into the main repository. In that case, in staging:oslo-2.

Thanks :-)

After revertig patches (http://qt.gitorious.org/qt/qt/commit/041a68007413a20a9a9c97d0f2f04f9e03428f67) this problem come back. Moving the mouse over links leave a trail with underlined links again. This bug need to be reopened.

(In reply to comment #16) > After revertig patches (http://qt.gitorious.org/qt/qt/commit/041a68007413a20a9a9c97d0f2f04f9e03428f67) this problem come back. Moving the mouse over links leave a trail with underlined links again. > > This bug need to be reopened. Actually I can only change it as won't fix. The graphics team is convinced there are too many inconsistency in the X11 specification to solve this problem in a reasonable time. Seeing all the subtle problems my patch introduced, I tend to also think that is not worth it.

Benjamin: well I believe we should still re-open the bug and fix it somehow - just leaving this as "wont fix" and leaving that ugly bug there sounds wrong to me.

(In reply to comment #18) > Benjamin: well I believe we should still re-open the bug and fix it somehow - just leaving this as "wont fix" and leaving that ugly bug there sounds wrong to me. Raster is becoming the default for X11 as well. I agree this is ugly but I also understand why the graphic team prefer to have other priorities.

(In reply to comment #19) Any way we can find a workaround for 4.7.x? I agree that this is rather ugly :)

Created attachment 68347 [details] Fix GraphicsContextQt::drawLine with X11 paint engine Hi all Qt4.7 release came, but the problem still exists. I'm trying to solve it... And I think what I found solution. Because the problem occured only in the webkit widgets for me (in other widgets, for example QTextEdit, i can't reproduce the problem), I think what webkit code must be corrected. In the method void GraphicsContext::adjustLineToPixelBoundaries(FloatPoint& p1, FloatPoint& p2, float strokeWidth, const StrokeStyle& penStyle), if strokeWidth is odd value, the coordinates increased by constant 0.5f and become outside context boundary rect. So, the replacement qRound to qFloor in the X11 painting system solves this problem, but generatesr probles. I desided use qFloor for point coordinates after calling adjustLineToPixelBoundaries(..) inside GraphicsContextQt::drawLine(...) if painter engine is X11 graphic engine. Attached patch do it and solve the problem.

keyboard... keyboard... "generatesr probles" must readed as "generates other problems" sorry :-)

Comment on attachment 68347 [details] Fix GraphicsContextQt::drawLine with X11 paint engine Does not sound bad. Suggestions: 1) If problem is still happen in trunk, make your patch against trunk; 2) add a ChangeLog to the patch: run $ prepare-ChangeLog --bug XXX ; 3) If you want that backported to QtWebKit2.0 or 2.1, provide a backported patch as well. 4) set r? and qt? to your patch.

(In reply to comment #21) > Qt4.7 release came, but the problem still exists. I'm trying to solve it... And I think what I found solution. > [...] I don't see how this is any better than the fix in X11. You'll have the exact same problem: the fill and outline won't be aligned on the grid. You can't just workaround bug of a graphics system in the upper layer. If you want to fix this, you have to it in the proper layer, in the graphics system X11.

(In reply to comment #24) > (In reply to comment #21) > > Qt4.7 release came, but the problem still exists. I'm trying to solve it... And I think what I found solution. > > [...] > > I don't see how this is any better than the fix in X11. You'll have the exact same problem: the fill and outline won't be aligned on the grid. > > You can't just workaround bug of a graphics system in the upper layer. If you want to fix this, you have to it in the proper layer, in the graphics system X11. Ok, Benjamin is a graphics specialist. I'd follow his advice. ps: > 4) set r? and qt? to your patch. doh! qt? = cq? :-)

Chatted with Benjamin, there is three ways to fix this bug: 1. Fix it in the X11 paint engine in Qt. This would require a lot of time to fine-tune all corner cases and will most likely break something else 2. Fix it in the Qt paint layer in QtWebKit when using the X11 paint engine. This has the same issues as way 1. 3. Fix it directly in the line drawing routing in WebCore in an #ifdef specific to Qt and the X11 paint engine. This would be the most efficient yet the most ugly layer jumping way of fixing it. So with all this ugliness and since this bug is not a regression but was unburrowed by the new Qt documentation, we thought that changing the new Qt documentation would be the best way to fix this problem. This would not fix other websites having this issue, but I'm not aware of any right now.

Created attachment 68387 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (thrid edition) Ok. > You can't just workaround bug of a graphics system in the upper layer. If you want to fix this, you have to it in the proper layer, > in the graphics system X11. I don't think so. Because point coordinates increased by 0.05f inside GraphicsContext::adjustLineToPixelBoundaries(...) before they will be passed to graphics engine. I have to admit that the unconditional flooring coordinates for all lines may have the exact same problem: the fill and outline won't be aligned on the grid, as Benjamin says. So we need two different methods to draw lines: for drawing normal lines (old behavior) and drawing lines for texts (new behaviour) or parametrise existing method. For example, change declaration of the method void GraphicsContext::drawLine(const IntPoint& point1, const IntPoint& point2); to void GraphicsContext::drawLine(const IntPoint& point1, const IntPoint& point2, bool forText = false) we added thrid parameter with default value FALSE, so we kept old behaviour. For Qt implementation of this method we will apply flooring of coordinates if forText is true and graphics engine is X11 -we will have new behavour. The forText parameter will be set to true in void GraphicsContext::drawLineForText(...) for Qt implementation, in other call places of drawLine() the value of forText parameter will be false an old behaviour will be kept. This (thrid :-)) patch flooring coordinates only line is drawing as underline with TextBox. I think that this would be the optimal solution.

(In reply to comment #23) > (From update of attachment 68347 [details]) > Does not sound bad. Suggestions: > > 1) If problem is still happen in trunk, make your patch against trunk; Yes. This problem occures in trunk. When we find optimal solution I make patch for trunk too. > 2) add a ChangeLog to the patch: run $ prepare-ChangeLog --bug XXX ; see 1) > 3) If you want that backported to QtWebKit2.0 or 2.1, provide a backported patch as well. I use WebKit inside Qt only. If you tell me how do it I do it. > 4) set r? and qt? to your patch. Explain please. I can't understand what is r? and cq?

(In reply to comment #27) > So we need two different methods to draw lines: for drawing normal lines (old behavior) and drawing lines for texts (new behaviour) or parametrise existing method. That would work, but you will need to convince the other ports this is a good idea because this is in common code. Once again, I am gonna state you should not workaround bugs. The right solution is to improve the graphics system X11.

Created attachment 68497 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fourth edition) ok. > That would work, but you will need to convince the other ports this is a good idea because this is in common code. Another solution is copying drawing code from GraphicsContext::drawLine(..) directly into GraphicsContext::drawLineForText(..) and make changes there for Qt implementation only. It's not affect other ports, because no common code has changed in this case. > Once again, I am gonna state you should not workaround bugs. The right solution is to improve the graphics system X11. Once again I will turn your attention what coordinates increase by 0.5f (if line thickness is odd) inside GraphicsContext::adjustLineToPixelBoundaries(..) and become outside of text bounding rect htere, before they will be passed to graphics engine. Improving the graphics system X11 is good idea, but "This would require a lot of time to fine-tune all corner cases and will most likely break something else" as Jocelyn Turcotte says. At this moment X11 graphics system works normal in most cases and this bug make using Qt 4.7 and higer very hard on X11-based platforms (IMHO likely even unusable) - predominantly on Linux. Most distributives already updated their packages of Qt... Last patch modify only GraphicsContext::drawLineForText(..) for Qt implementation if Qt has compiled for X11 only and X11 graphics system is used (at runtime).

(In reply to comment #26) > So with all this ugliness and since this bug is not a regression but was unburrowed by the new Qt documentation, we thought that changing the new Qt documentation would be the best way to fix this problem. > > This would not fix other websites having this issue, but I'm not aware of any right now. This problem hapen on most HTML document where margin between anchors becomes to zero via CSS, not Qt documentation only.. If anchors put without spaces from eachother verticaly, artifacts hapen when mouse move from bottom to top over these anchors.

Hey Guys. Having wontfix here leaves QWebKit unusable on a large number of installations. This is not a good idea. Even ugly fix is better than no fix at all. I'd prefer temporary ugly fix now and good fix later.

(In reply to comment #32) > Hey Guys. Having wontfix here leaves QWebKit unusable on a large number of installations. This is not a good idea. Even ugly fix is better than no fix at all. I'd prefer temporary ugly fix now and good fix later. I am a bit surprised by the interest on this bug, especially since it is _NOT_ a regression. Most apps I use that are based on QtWebKit already force the graphics system raster (and at least the last versions for Rekonq and Arora both force raster). So those do not encounter the issue. Could you please explain which use case gave this bug visibility?

(In reply to comment #33) > (In reply to comment #32) > > Hey Guys. Having wontfix here leaves QWebKit unusable on a large number of installations. This is not a good idea. Even ugly fix is better than no fix at all. I'd prefer temporary ugly fix now and good fix later. > > I am a bit surprised by the interest on this bug, especially since it is _NOT_ a regression. > > Most apps I use that are based on QtWebKit already force the graphics system raster (and at least the last versions for Rekonq and Arora both force raster). So those do not encounter the issue. > But I don't use Rekonq and Arora :( I just have ran these programs to look how they work. > Could you please explain which use case gave this bug visibility? I use linux desktop with KDE4 as office workstation and qtcreator for software development. If X11 graphics engine is selected I have ugly artefacts in a lot of programs, very annoying artefacts in qtcreator (and assistant) and browsers. I agree, if raster graphics engine is selected, the artifacts not apear and most progams look fine. But I have another problem in this case... When I try start OpenOffice.org It crashes with message: X-Error: BadDrawable (invalid Pixmap or Window parameter) Major opcode: 62 (X_CopyArea) Resource ID: 0x0 Serial No: 510 (510) (maybe it is this bug http://bugreports.qt.nokia.com/browse/QTBUG-7702?page=com.atlassian.jira.plugin.system.issuetabpanels%3Aall-tabpanel) Patch each program which I use to using raster graphics engine or make wrappers is expensive over time and not very convenient. Enabling raster graphics engine as global, adds problem in other programs. So, I ask you fix (or make workaraund) this problem. I suggested a solution, ofcourse the solution isn't ideal but it may close the problem.

(In reply to comment #34) > So, I ask you fix (or make workaraund) this problem. I suggested a solution, ofcourse the solution isn't ideal but it may close the problem. I am still not convinced by the patch but you might be able to convince a reviewer. If you want the patch to be considered for review, you should follow https://bugs.webkit.org/show_bug.cgi?id=42248#c23 (some more doc here: http://trac.webkit.org/wiki/QtWebKitContrib) Please add a comment in the duplicated code to explain why we need such a ugly workaround. Please also add a #ifdef for the Qt version: #if !defined(Q_WS_X11) || (QT_VERSION >= QT_VERSION_CHECK(4, 8, 0)) Raster is the default starting with 4.8. With such a check, the duplicated code can be removed eventually.

(In reply to comment #35) > (In reply to comment #34) > If you want the patch to be considered for review, you should follow https://bugs.webkit.org/show_bug.cgi?id=42248#c23 (some more doc here: http://trac.webkit.org/wiki/QtWebKitContrib) In Qt webkit code no script prepare-ChangeLog. When I try clone WebKit from gitorious.org I get error: $ git clone git://gitorious.org/webkit/webkit.git Cloning into webkit... remote: Counting objects: 877230, done. fatal: The remote end hung up unexpectedly fatal: early EOF fatal: index-pack failed

(In reply to comment #36) > In Qt webkit code no script prepare-ChangeLog. When I try clone WebKit from gitorious.org I get error: > > $ git clone git://gitorious.org/webkit/webkit.git > Cloning into webkit... > remote: Counting objects: 877230, done. > fatal: The remote end hung up unexpectedly > fatal: early EOF > fatal: index-pack failed Hum, try again. I had problem recently with gitorious and I had to try several times before the clone succeed.

(In reply to comment #37) > (In reply to comment #36) > > In Qt webkit code no script prepare-ChangeLog. When I try clone WebKit from gitorious.org I get error: > > > > $ git clone git://gitorious.org/webkit/webkit.git > > Cloning into webkit... > > remote: Counting objects: 877230, done. > > fatal: The remote end hung up unexpectedly > > fatal: early EOF > > fatal: index-pack failed > > Hum, try again. I had problem recently with gitorious and I had to try several times before the clone succeed. See also http://qtwebkit.blogspot.com/2010/09/fatal-remote-end-hung-up-unexpectedly.html Simply use ssh to pull webkit from gitorious.

Created attachment 70391 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition) The last edition of the patch. I deside to make minimal code for workaround of this bug. The result is equivalent to changes in fourth edition of patch. In X11 version of Qt, X11 painting engine may be set as default by option -graphicssystem with configure script. Then the problem with artefacts will be appered again, so I deside don't add "(QT_VERSION >= QT_VERSION_CHECK(4, 8, 0))" to #ifdef condition. NOTE: This patch make workaround only fof X11 version of Qt and only if X11 paint engine is used.

Created attachment 70392 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, corrected) Some corrections. Why I can't reload or delete patches which I uploaded?

Comment on attachment 70391 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition) View in context: https://bugs.webkit.org/attachment.cgi?id=70391&action=review nit picking: > WebCore/platform/graphics/qt/GraphicsContextQt.cpp:849 > + // if paintengine type is X11 to avoid artifacts comments start with capital and end with "." > WebCore/platform/graphics/qt/GraphicsContextQt.cpp:853 > + // If stroke thiknes is odd we need decrease Y coordinate by 1, thiknes -> typo > WebCore/platform/graphics/qt/GraphicsContextQt.cpp:857 > + // integer value period (".") > WebCore/platform/graphics/qt/GraphicsContextQt.cpp:859 > + if (static_cast<int>(strokeWidth) % 2) { //odd drop the comment.

Created attachment 70396 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, corrected) Updated

Comment on attachment 70396 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, corrected) View in context: https://bugs.webkit.org/attachment.cgi?id=70396&action=review more nit picking (sorry did not catch them at first time) > WebCore/platform/graphics/qt/GraphicsContextQt.cpp:852 > + // If stroke thickness is odd we need decrease Y coordinate by 1, > + // because inside methotod adjustLineToPixelBoundaries(...), which 1 pixel? please specify typo -> methotod

Created attachment 70397 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, corrected) Updated again ;-)

Comment on attachment 70397 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, corrected) View in context: https://bugs.webkit.org/attachment.cgi?id=70397&action=review > WebCore/platform/graphics/qt/GraphicsContextQt.cpp:840 > +#if !defined(Q_WS_X11) This define seems a bit useless. It will always be defined anyway for MeeGo, Maemo etc even if these platforms use raster or opengl > WebCore/platform/graphics/qt/GraphicsContextQt.cpp:845 > + IntPoint p1 = origin; > + IntPoint p2 = origin + IntSize(width, 0); Why not startPoint, endPoint

(In reply to comment #45) > (From update of attachment 70397 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=70397&action=review > > > WebCore/platform/graphics/qt/GraphicsContextQt.cpp:840 > > +#if !defined(Q_WS_X11) > > This define seems a bit useless. It will always be defined anyway for MeeGo, Maemo etc even if these platforms use raster or opengl You still have Windows, Mac OS X, WinCe, etc Actually, what about having the code without #if-#else and just add a guard around the if()?: + IntPoint p1 = origin; + IntPoint p2 = origin + IntSize(width, 0); + + // If paintengine type is X11 to avoid artifacts + // like bug https://bugs.webkit.org/show_bug.cgi?id=42248 +#if !defined(Q_WS_X11) + QPainter* p = m_data->p(); + if (p->paintEngine()->type() == QPaintEngine::X11) { + // If stroke thickness is odd we need decrease Y coordinate by 1 pixel, + // because inside method adjustLineToPixelBoundaries(...), which + // called from drawLine(...), Y coordinate will be increased by 0.5f + // and then inside Qt painting engine will be rounded to next greater + // integer value. + float strokeWidth = strokeThickness(); + if (static_cast<int>(strokeWidth) % 2) { + p1.setY(p1.y() - 1); + p2.setY(p2.y() - 1); + } + } +#endif // !defined(Q_WS_X11) + + drawLine(p1, p2); } I actually start to think this patch does not look so bad after all. The special path is not too complex, and it has good comments.

I meant +#if defined(Q_WS_X11)

Created attachment 70400 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, corrected) (In reply to comment #45) > (From update of attachment 70397 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=70397&action=review > > > WebCore/platform/graphics/qt/GraphicsContextQt.cpp:840 > > +#if !defined(Q_WS_X11) > > This define seems a bit useless. It will always be defined anyway for MeeGo, Maemo etc even if these platforms use raster or opengl I changed condition to !(defined(Q_WS_X11) && defined(Q_OS_UNIX)) It's more exactly? Even this condition was useless, then I check for garaphics engine is X11. If graphics engine is raster or opengl then no changes will be made - the original behavior will be kept > > WebCore/platform/graphics/qt/GraphicsContextQt.cpp:845 > > + IntPoint p1 = origin; > > + IntPoint p2 = origin + IntSize(width, 0); > > Why not startPoint, endPoint It not fundamental, I made it.

(In reply to comment #46) > (In reply to comment #45) > > Actually, what about having the code without #if-#else and just add a guard around the if()?: Do you mean like that: void GraphicsContext::drawLineForText(const IntPoint& origin, int width, bool) { if (paintingDisabled()) return; IntPoint startPoint = origin; IntPoint endPoint = origin + IntSize(width, 0); // If paintengine type is X11 to avoid artifacts // like bug https://bugs.webkit.org/show_bug.cgi?id=42248 QPainter* p = m_data->p(); if (p->paintEngine()->type() == QPaintEngine::X11) { // If stroke thickness is odd we need decrease Y coordinate by 1 pixel, // because inside method adjustLineToPixelBoundaries(...), which // called from drawLine(...), Y coordinate will be increased by 0.5f // and then inside Qt painting engine will be rounded to next greater // integer value. float strokeWidth = strokeThickness(); if (static_cast<int>(strokeWidth) % 2) { startPoint.setY(startPoint.y() - 1); endPoint.setY(endPoint.y() - 1); } } drawLine(startPoint, endPoint); }

> Actually, what about having the code without #if-#else and just add a guard around the if()?: > > > + IntPoint p1 = origin; > + IntPoint p2 = origin + IntSize(width, 0); > + > + // If paintengine type is X11 to avoid artifacts > + // like bug https://bugs.webkit.org/show_bug.cgi?id=42248 > +#if !defined(Q_WS_X11) > + QPainter* p = m_data->p(); > + if (p->paintEngine()->type() == QPaintEngine::X11) { > + // If stroke thickness is odd we need decrease Y coordinate by 1 pixel, > + // because inside method adjustLineToPixelBoundaries(...), which > + // called from drawLine(...), Y coordinate will be increased by 0.5f > + // and then inside Qt painting engine will be rounded to next greater > + // integer value. > + float strokeWidth = strokeThickness(); > + if (static_cast<int>(strokeWidth) % 2) { > + p1.setY(p1.y() - 1); > + p2.setY(p2.y() - 1); > + } > + } > +#endif // !defined(Q_WS_X11) > + > + drawLine(p1, p2); > > } > > > I actually start to think this patch does not look so bad after all. The special path is not too complex, and it has good comments. This I'm all for.

Comment on attachment 70400 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, corrected) r-, let's go with Benjamin's suggestion

Created attachment 70403 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, wuthout #if)

Comment on attachment 70403 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, wuthout #if) View in context: https://bugs.webkit.org/attachment.cgi?id=70403&action=review > WebCore/platform/graphics/qt/GraphicsContextQt.cpp:845 > + // like bug https://bugs.webkit.org/show_bug.cgi?id=42248 > + QPainter* p = m_data->p(); Benjamin, asked you to keep the ifdef, but add it here just before the QPainter, like #if defined(Q_WS_X11). Check his comment for more info. He even showed the code.

Created attachment 70404 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, wuth moved #if) Sorry, I was inattentive... I corrected path as Benjamin asks

Comment on attachment 70404 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, wuth moved #if) Rejecting patch 70404 from commit-queue. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'land-attachment', '--force-clean', '--ignore-builders', '--quiet', '--non-interactive', '--parent-command=commit-queue', 70404]" exit_code: 2 Last 500 characters of output: /svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/platform/graphics/qt/GraphicsContextQt.cpp A repository hook failed: Commit blocked by pre-commit hook (exit code 1) with output: svnlook: Can't write to stream: Broken pipe The following ChangeLog files contain OOPS: trunk/WebCore/ChangeLog Please don't ever say "OOPS" in a ChangeLog file. at /usr/local/git/libexec/git-core/git-svn line 572 Failed to run "[u'git', u'svn', u'dcommit']" exit_code: 1 Full output: http://queues.webkit.org/results/4339014

Created attachment 70421 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, wuth moved #if) Removed any 'OOPS' from changelog. But at http://trac.webkit.org/wiki/QtWebKitContrib written "Keep the "Reviewed by NOBODY (OOPS!)." line in the ChangeLog files, it will be edited by the bot or person that will commit your fix. " It's correct?

Comment on attachment 70404 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, wuth moved #if) Cleared Kenneth Rohde Christiansen's review+ from obsolete attachment 70404 [details] so that this bug does not appear in http://webkit.org/pending-commit.

Comment on attachment 70421 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, wuth moved #if) View in context: https://bugs.webkit.org/attachment.cgi?id=70421&action=review Some nitpicking. Do not forget got put the flags to "?" when you add a patch, otherwise we don't see it is needing a review. > WebCore/platform/graphics/qt/GraphicsContextQt.cpp:57 > +#include <QtCore> Please include qglobal instead. Including QtCore means including all the headers. > WebCore/platform/graphics/qt/GraphicsContextQt.cpp:845 > +#if defined(Q_WS_X11) && defined(Q_OS_UNIX) Please remove the "&& defined(Q_OS_UNIX)". Some people are using X11 on Windows as well (I know, it is a strange use case :)).

Created attachment 70947 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, wuth moved #if) (In reply to comment #58) > (From update of attachment 70421 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=70421&action=review > > Some nitpicking. Do not forget got put the flags to "?" when you add a patch, otherwise we don't see it is needing a review. > > > WebCore/platform/graphics/qt/GraphicsContextQt.cpp:57 > > +#include <QtCore> QtCore no more needed. Removed. > > Please include qglobal instead. Including QtCore means including all the headers. > > > WebCore/platform/graphics/qt/GraphicsContextQt.cpp:845 > > +#if defined(Q_WS_X11) && defined(Q_OS_UNIX) > Done.

Comment on attachment 70947 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, wuth moved #if) Rejecting patch 70947 from commit-queue. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'land-attachment', '--force-clean', '--ignore-builders', '--quiet', '--non-interactive', '--parent-command=commit-queue', 70947]" exit_code: 1 Last 500 characters of output: tachment.cgi?id=70947&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=42248&ctype=xml Processing 1 patch from 1 bug. Cleaning working directory Updating working directory Processing patch 70947 from bug 42248. NOBODY found in /Projects/CommitQueue/WebCore/ChangeLog does not appear to be a valid reviewer according to committers.py. ERROR: /Projects/CommitQueue/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/4446054

Created attachment 70960 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, wuth moved #if) > ERROR: /Projects/CommitQueue/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Done.

Comment on attachment 70960 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, wuth moved #if) View in context: https://bugs.webkit.org/attachment.cgi?id=70960&action=review > WebCore/ChangeLog:3 > + Unreviewed. You should leave "Reviewed by NOBODY (OOPS!)" and the commitbot should fill it for you properly. "Unreviewed" is essentially for build fixes.

Created attachment 70969 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, wuth moved #if) > You should leave "Reviewed by NOBODY (OOPS!)" and the commitbot should fill it for you properly. > > "Unreviewed" is essentially for build fixes. Return back "Reviewed by NOBODY (OOPS!)". I've removed "Reviewed by NOBODY (OOPS!)" after I get following message. (In reply to comment #55) > (From update of attachment 70404 [details]) > Rejecting patch 70404 from commit-queue. > > Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'land-attachment', '--force-clean', '--ignore-builders', '--quiet', '--non-interactive', '--parent-command=commit-queue', 70404]" exit_code: 2 > Last 500 characters of output: > /svn.webkit.org/repository/webkit/trunk ... > M WebCore/ChangeLog > M WebCore/platform/graphics/qt/GraphicsContextQt.cpp > A repository hook failed: Commit blocked by pre-commit hook (exit code 1) with output: > svnlook: Can't write to stream: Broken pipe > > The following ChangeLog files contain OOPS: > > trunk/WebCore/ChangeLog > > Please don't ever say "OOPS" in a ChangeLog file. > at /usr/local/git/libexec/git-core/git-svn line 572 > > > Failed to run "[u'git', u'svn', u'dcommit']" exit_code: 1 > > Full output: http://queues.webkit.org/results/4339014

Comment on attachment 70969 [details] Fix GraphicsContextQt::drawLine with X11 paint engine (fifth edition, wuth moved #if) Clearing flags on attachment: 70969 Committed r69923: <http://trac.webkit.org/changeset/69923>

All reviewed patches have been landed. Closing bug.

*** Bug 45499 has been marked as a duplicate of this bug. ***