[Webkit-unassigned] [Bug 29803] [Qt] QWebHistory and QWebPage autotests crash

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 26 20:06:12 PDT 2009


https://bugs.webkit.org/show_bug.cgi?id=29803





--- Comment #27 from Holger Freyther <zecke at selfish.org>  2009-10-26 20:06:11 PDT ---
(In reply to comment #25)

(reordered for importance)

> > I think the API contract should be changed to not emit frameCreated from the
> > QWebPage c'tor and that it is guranteed that the mainFrame() exists...
> This is an awesome catch! Thanks. Because of that the solution is completely
> bad. The only workarounds I can find for the problem are based on
> MagicSignalRouter(tm) class or on some operation on the EventLoop. This would
> be evil and an overkill.
> So I believe we should leave current solution. It is more readable and it will
> be easier to maintenance.

hehe, I like the killing of createMainFrame in your solution a lot. I will try
to poke Simon once he is back from vacation.


> > why did you remove this space?
> <BadJoke> I removed these lines to speed up compilation </BadJoke>
> I've just removed an empty line... Why it is bad? Why the line should be there?
> Why you say it is a bogus change?
> Really, the lines are meaningless and actually I think they are confusing, an
> empty lines should divide some logical blocks, but this isn't the case. Since I
> had to modify the part of source code I decided to reformat it.

Right they are meaningless, why do you change them then? The main point is that
you propose to fix a specific problem and when the patch contains unrelated
things it gets harder to review. As a reviewer you will have to ask yourself if
the contributor is using a Yedi mind trick to fool you or what he tries to
hide.

The other part is that if one is not enforcing a conherent coding style one
ends up having a mess like the symbian kernel where {} are freely placed with
every kind of indention.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list