Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#14599 closed enhancement (fixed)

Add styles support for links in wxHtml

Reported by: gevorg Owned by:
Priority: normal Milestone:
Component: wxHtml Version: stable-latest
Keywords: html links href style a Cc:
Blocked By: #14443, #14443, #14443, #14443, #14443, #14443, #14443, #14443 Blocking:
Patch: yes

Description

Processing of style attribute in <a> tag is currently missing. The attached patch adds it. #14443 needs to be committed before this patch can be applied.

Attachments (3)

html_link_style.patch download (10.9 KB) - added by gevorg 2 years ago.
#14443 needs to be committed before this patch can be applied, as it uses background color/mode related functions that are added in http://trac.wxwidgets.org/attachment/ticket/14443/span_background_color.1.patch
html_link_style.1.patch download (11.4 KB) - added by gevorg 2 years ago.
Update the patch now that #14443 got fixed
html_dc_bg_mode_not_brushstyle.patch download (2.6 KB) - added by gevorg 2 years ago.
Use wx{SOLID,TRANSPARENT} instead of wxBRUSHSTYLE_{SOLID,TRANSPARENT} in the remaining appropriate places throughout wxHtml

Download all attachments as: .zip

Change History (12)

Changed 2 years ago by gevorg

#14443 needs to be committed before this patch can be applied, as it uses background color/mode related functions that are added in http://trac.wxwidgets.org/attachment/ticket/14443/span_background_color.1.patch

comment:1 Changed 2 years ago by vadz

  • Blocked By

(In #14443) Could you please add an example of use of this tag to some of the HTML files in samples/html/test please to make it possible to easily check that it does indeed work correctly?

comment:2 Changed 2 years ago by gevorg

  • Blocked By

(In #14443) Replying to vadz:

Could you please add an example of use of this tag to some of the HTML files in samples/html/test please to make it possible to easily check that it does indeed work correctly?

Sure. Below is the updated patch.

comment:3 Changed 2 years ago by vadz

  • Blocked By

(In #14443) Thanks for the update and sorry for the delay with reviewing the patch!

Now that I finally did it I realize that I'm not entirely happy with it unfortunately. The confusing thing is that adds a new wxHtmlColourCell ctor taking wxBrushStyle and then uses this for both creating the brush with this style and calling wxDC::SetBackgroundMode() with it. The latter does work because wxBRUSHSTYLE_SOLID == wxSOLID and wxBRUSHSTYLE_TRANSPARENT == wxTRANSPARENT but I'm uncomfortable relying on this. The former bothers me too as creating a transparent brush is entirely unnecessary when the background mode is transparent -- the background brush is not used at all in this case.

Also, the existing 2 argument wxHtmlColourCell ctor is documented so it actually is part of the public API (and it does make sense, you could use it as a base class for your custom cell class) and so shouldn't be modified in incompatible way as the patch does by removing its second argument.

So I think the best would be to actually just add some new wxHTML_CLR_OPAQUE_BACKGROUND (or wxHTML_CLR_TRANSPARENT_BACKGROUND? I'm a bit hazy about which one does the current wxHTML_CLR_BACKGROUND correspond to, but the idea is to add another constant for the other kind of background) and call wxDC::SetBackgroundMode() accordingly and not create the background brush unnecessarily.

Would it be possible for you to modify the patch like this if you don't see any problems with this approach?

Thanks again!

comment:4 Changed 2 years ago by gevorg

  • Blocked By

(In #14443) Replying to vadz:

Thanks for the update and sorry for the delay with reviewing the patch!

Now that I finally did it I realize that I'm not entirely happy with it unfortunately. The confusing thing is that adds a new wxHtmlColourCell ctor taking wxBrushStyle and then uses this for both creating the brush with this style and calling wxDC::SetBackgroundMode() with it. The latter does work because wxBRUSHSTYLE_SOLID == wxSOLID and wxBRUSHSTYLE_TRANSPARENT == wxTRANSPARENT but I'm uncomfortable relying on this. The former bothers me too as creating a transparent brush is entirely unnecessary when the background mode is transparent -- the background brush is not used at all in this case.

Also, the existing 2 argument wxHtmlColourCell ctor is documented so it actually is part of the public API (and it does make sense, you could use it as a base class for your custom cell class) and so shouldn't be modified in incompatible way as the patch does by removing its second argument.

So I think the best would be to actually just add some new wxHTML_CLR_OPAQUE_BACKGROUND (or wxHTML_CLR_TRANSPARENT_BACKGROUND? I'm a bit hazy about which one does the current wxHTML_CLR_BACKGROUND correspond to, but the idea is to add another constant for the other kind of background) and call wxDC::SetBackgroundMode() accordingly and not create the background brush unnecessarily.

Would it be possible for you to modify the patch like this if you don't see any problems with this approach?

Thanks again!

Thanks for your feedback and sorry for the slow reply!
Didn't see that wxHtmlColourCell ctor was documented, thanks for pointing that out!
Please have a look at the new patch and let me know if it's close to what you had in mind.

comment:5 Changed 2 years ago by gevorg

  • Blocked By

(In #14443) Just realized that I still didn't address your comment on using wxBrushStyle for calling wxDC::SetBackgroundMode(). I can fix that as well, if everything else is OK.

comment:6 Changed 2 years ago by VZ

  • Blocked By

(In #14443) (In [72589]) Add support for background-color style to span element in wxHTML.

Add code for setting/restoring background mode and use it to implement support
for changing the text background colour.

Closes #14443.

Changed 2 years ago by gevorg

Update the patch now that #14443 got fixed

Changed 2 years ago by gevorg

Use wx{SOLID,TRANSPARENT} instead of wxBRUSHSTYLE_{SOLID,TRANSPARENT} in the remaining appropriate places throughout wxHtml

comment:7 Changed 2 years ago by VZ

  • Resolution set to fixed
  • Status changed from new to closed

(In [72630]) Support some CSS styles for the links in wxHTML too.

Refactor limited CSS styles support for <span> tag to reuse it for <a> tag as
well.

Closes #14599.

comment:8 Changed 2 years ago by VZ

(In [72631]) No real changes, just don't use brush styles for background mode in wxHTML.

Use just wxTRANSPARENT and wxSOLID instead of wxBRUSHSTYLE_TRANSPARENT and
wxBRUSHSTYLE_SOLID when changing the background mode.

See #14599.

comment:9 Changed 2 years ago by VZ

  • Blocked By

(In #14443) (In [73393]) Really fix the background colour used for the cells in wxHTML tables.

Respect "bgcolor" attributes of <td> tags, they were ignored since the changes
of r72589 (see #14443).

Closes #14909.

Note: See TracTickets for help on using tickets.