[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