WML <card> title attribute is not bieng implemented in WMLCardElement as a result it fails to show card title on the title bar of GtkLauncher. This implementation is returning empty string // Update the document title doc->setTitle(activeCard->title());
Created attachment 31402 [details] Patch to add title attribute to card element Adding title attribute to WML <card> element with necessary layout test
Created attachment 31404 [details] Patch to add title attribute to card element Resubmitting with a review flag Adding title attribute to WML <card> element with necessary layout test
Comment on attachment 31404 [details] Patch to add title attribute to card element Hi Wajat, thanks for the patch. Good that you spotted it, though I'd suggest a better implementation of that. WMLCardElement already provides a virtual-function "String title() const" through the Element parent class. So all you'd need to do is implement WMLCardElement::title() and have it return "parseValueSub...(getAttribute(titleAttr...". See HTMLElement::title() for comparision. This way we save memory. Glad if you could upload a new patch. Niko
Created attachment 31540 [details] Updated Patch Hi Niko, thanks for the comments. Updated as per your suggestions. Performing const_cast to remove the const property. I hope this approach is good. let me know your comments.
Comment on attachment 31540 [details] Updated Patch I think this is meant to be r? rather than r+. Setting it for review.
Comment on attachment 31540 [details] Updated Patch Looks good except the const_cast. Just make the parseValueSubstitutingVariableReferences & parseValueForbidding... functions const. They're not modifying any member variables and they are non-const by mistake. Can you upload a new patch? As you don't have commit access I'm going to clear the review flag until a new patch has been uploaded. Going to commit that patch then.
Created attachment 31633 [details] updated patch to fix const_cast modified as per Niko's comment#6 by making parseValueSubstitutingVariableReferences & parseValueForbidding... as const intead of performing const_cast. Pl. give a final review and get this in. thanks Wajahat
Comment on attachment 31633 [details] updated patch to fix const_cast How do we test this? Your ChangeLog mentions a layout test, but it seems to be missing from your patch. Otherwise this looks fine... but I don't understand how this method is ever used or tested?
Created attachment 31635 [details] Adding missed manual test resubmitting patch by Adding manual/layout test, for testing this patch
Comment on attachment 31635 [details] Adding missed manual test Looks good! r=me.
(In reply to comment #10) > (From update of attachment 31635 [details] [review]) > Looks good! r=me. > thanks Niko can you get this in. regards
Landed in r45081. Closing bug.